From f21e74d0e743e542b272f8de267c8d4e6ae84ad4 Mon Sep 17 00:00:00 2001
From: "mathias.chouet" <mathias.chouet@irstea.fr>
Date: Fri, 18 Oct 2019 16:53:28 +0200
Subject: [PATCH] Fix #155 - stop chain calculation if an error occurs

add logs in downstream Nubs when non-fatal errors occur
---
 spec/param/param_var.spec.ts                  |  2 +-
 spec/solveur/solveur.spec.ts                  | 87 +++++++++++++++----
 .../value_ref_chained_computation.spec.ts     | 75 +++++++++++++++-
 src/nub.ts                                    | 56 ++++++++++--
 src/session.ts                                |  9 ++
 src/util/log.ts                               | 15 +++-
 src/util/message.ts                           | 14 ++-
 src/util/result.ts                            | 18 +++-
 8 files changed, 244 insertions(+), 32 deletions(-)

diff --git a/spec/param/param_var.spec.ts b/spec/param/param_var.spec.ts
index a4311077..99e80383 100644
--- a/spec/param/param_var.spec.ts
+++ b/spec/param/param_var.spec.ts
@@ -20,7 +20,7 @@ describe("when a parameter is varying, ", () => {
             expect(res.resultElements[4].log.messages.length).toBe(1);
             // check global log
             expect(res.globalLog.messages.length).toBe(1);
-            expect(res.globalLog.messages[0].code).toBe(MessageCode.ERROR_ABSTRACT);
+            expect(res.globalLog.messages[0].code).toBe(MessageCode.WARNING_ERRORS_ABSTRACT);
             expect(res.globalLog.messages[0].extraVar.nb).toBe("3"); // number is a string here
         });
     });
diff --git a/spec/solveur/solveur.spec.ts b/spec/solveur/solveur.spec.ts
index 680d44a9..639f21f3 100644
--- a/spec/solveur/solveur.spec.ts
+++ b/spec/solveur/solveur.spec.ts
@@ -19,9 +19,7 @@ describe("solveur multi-modules", () => {
         const pp = new PabPuissance(new PabPuissanceParams(666, 0.1, 0.5, 666));
         pp.calculatedParam = pp.prms.PV;
         Session.getInstance().clear();
-        Session.getInstance().registerNub(pc);
-        Session.getInstance().registerNub(pn);
-        Session.getInstance().registerNub(pp);
+        Session.getInstance().registerNubs([ pc, pn, pp ]);
         pn.prms.DHT.defineReference(pc, "DH");
         pp.prms.DH.defineReference(pn, "DH");
         // solveur
@@ -42,9 +40,7 @@ describe("solveur multi-modules", () => {
         const pp = new PabPuissance(new PabPuissanceParams(666, 0.1, 0.5, 666));
         pp.calculatedParam = pp.prms.PV;
         Session.getInstance().clear();
-        Session.getInstance().registerNub(pc);
-        Session.getInstance().registerNub(pn);
-        Session.getInstance().registerNub(pp);
+        Session.getInstance().registerNubs([ pc, pn, pp ]);
         pn.prms.DHT.defineReference(pc, "DH");
         pp.prms.DH.defineReference(pn, "DH");
         // solveur
@@ -64,9 +60,7 @@ describe("solveur multi-modules", () => {
         const pp = new PabPuissance(new PabPuissanceParams(666, 0.1, 0.5, 666));
         pp.calculatedParam = pp.prms.PV;
         Session.getInstance().clear();
-        Session.getInstance().registerNub(pc);
-        Session.getInstance().registerNub(pn);
-        Session.getInstance().registerNub(pp);
+        Session.getInstance().registerNubs([ pc, pn, pp ]);
         // solveur
         const s = new Solveur(new SolveurParams(324.907), false);
         s.searchedParameter = pc.prms.Z1;
@@ -84,9 +78,7 @@ describe("solveur multi-modules", () => {
         const pp = new PabPuissance(new PabPuissanceParams(666, 0.1, 0.5, 666));
         pp.calculatedParam = pp.prms.PV;
         Session.getInstance().clear();
-        Session.getInstance().registerNub(pc);
-        Session.getInstance().registerNub(pn);
-        Session.getInstance().registerNub(pp);
+        Session.getInstance().registerNubs([ pc, pn, pp ]);
         pn.prms.DHT.defineReference(pc, "DH");
         pp.prms.DH.defineReference(pn, "DH");
         // solveur
@@ -110,9 +102,7 @@ describe("solveur multi-modules", () => {
         const pp = new PabPuissance(new PabPuissanceParams(666, 0.1, 0.5, 666));
         pp.calculatedParam = pp.prms.PV;
         Session.getInstance().clear();
-        Session.getInstance().registerNub(pc);
-        Session.getInstance().registerNub(pn);
-        Session.getInstance().registerNub(pp);
+        Session.getInstance().registerNubs([ pc, pn, pp ]);
         pn.prms.DHT.defineReference(pc, "DH");
         pp.prms.DH.defineReference(pn, "DH");
         // solveur
@@ -137,9 +127,7 @@ describe("solveur multi-modules", () => {
         const pp = new PabPuissance(new PabPuissanceParams(666, 0.1, 0.5, 666));
         pp.calculatedParam = pp.prms.PV;
         Session.getInstance().clear();
-        Session.getInstance().registerNub(pc);
-        Session.getInstance().registerNub(pn);
-        Session.getInstance().registerNub(pp);
+        Session.getInstance().registerNubs([ pc, pn, pp ]);
         pn.prms.DHT.defineReference(pc, "DH");
         pp.prms.DH.defineReference(pn, "DH");
         // check lists
@@ -153,4 +141,67 @@ describe("solveur multi-modules", () => {
         expect(searchedParamsPn.map((p) => p.symbol)).toEqual([ "Z1", "Z2" ]);
     });
 
+    it("test 7: suppression des extraResult", () => {
+        // contexte
+        const pc = new PabChute(new PabChuteParams(2, 0.5, 666));
+        pc.calculatedParam = pc.prms.DH;
+        const pn = new PabNombre(new PabNombreParams(666, 10, 666));
+        pn.calculatedParam = pn.prms.DH;
+        Session.getInstance().clear();
+        Session.getInstance().registerNubs([ pc, pn ]);
+        pn.prms.DHT.defineReference(pc, "DH");
+        // solveur
+        const s = new Solveur(new SolveurParams(12.5), false);
+        s.nubToCalculate = pn;
+        s.searchedParameter = pc.prms.Z1;
+        const res = s.CalcSerie();
+        expect(res.vCalc).toBeCloseTo(125.5, 1);
+        expect(Object.keys(res.resultElement.values).length).toBe(1);
+        expect(res.resultElement.vCalcSymbol).toBe("X");
+    });
+
+    it("test 8: vCalcSymbol du Nub calculé", () => {
+        // contexte
+        const pc = new PabChute(new PabChuteParams(2, 0.5, 666));
+        pc.calculatedParam = pc.prms.DH;
+        const pn = new PabNombre(new PabNombreParams(666, 10, 666));
+        pn.calculatedParam = pn.prms.DH;
+        Session.getInstance().clear();
+        Session.getInstance().registerNubs([ pc, pn ]);
+        pn.prms.DHT.defineReference(pc, "DH");
+        // solveur
+        const s = new Solveur(new SolveurParams(12.5), false);
+        s.nubToCalculate = pn;
+        s.searchedParameter = pc.prms.Z1;
+        const res = s.CalcSerie();
+        expect(res.resultElement.vCalcSymbol).toBe("X");
+        expect(pn.result.resultElement.vCalcSymbol).toBe("DH");
+    });
+
+    // @TODO
+    xit("test 9: remonter les logs en cas d'erreur dans la chaîne de calcul", () => {
+        // contexte
+        const pc = new PabChute(new PabChuteParams(2, 0.5, 666));
+        pc.calculatedParam = pc.prms.DH;
+        const pn = new PabNombre(new PabNombreParams(666, 10, 666));
+        pn.calculatedParam = pn.prms.DH;
+        const pp = new PabPuissance(new PabPuissanceParams(666, 0.1, 0.5, 666));
+        pp.calculatedParam = pp.prms.PV;
+        Session.getInstance().clear();
+        Session.getInstance().registerNubs([ pc, pn, pp ]);
+        pn.prms.DHT.defineReference(pc, "DH");
+        pp.prms.DH.defineReference(pn, "DH");
+        // solveur
+        const s = new Solveur(new SolveurParams(1), false);
+        s.nubToCalculate = pp;
+        s.searchedParameter = pc.prms.Z1;
+        const res = s.CalcSerie();
+        expect(res.vCalc).toBeCloseTo(2.156, 3);
+    });
+
+    // @TODO si la dicho ne converge pas, indiquer la dernière valeur testée
+
+    // @TODO chargement de session avec Solveur (2 passes)
+
+    // @TODO calcul en chaîne: si une étape foire, interrompre et faire remonter l'erreur
 });
diff --git a/spec/value_ref/value_ref_chained_computation.spec.ts b/spec/value_ref/value_ref_chained_computation.spec.ts
index 21a320f1..d5fee01c 100644
--- a/spec/value_ref/value_ref_chained_computation.spec.ts
+++ b/spec/value_ref/value_ref_chained_computation.spec.ts
@@ -1,5 +1,5 @@
 import { Cloisons, CreateStructure, cSnRectang, LoiDebit,
-         SectionParametree, Session } from "../../src/index";
+         MessageCode, SectionParametree, Session } from "../../src/index";
 import { CloisonsParams } from "../../src/pab/cloisons_params";
 import { PabChute } from "../../src/pab/pab_chute";
 import { PabChuteParams } from "../../src/pab/pab_chute_params";
@@ -136,3 +136,76 @@ describe("chained computation of linked Nubs : ", () => {
         expect(r1).not.toBe(r2);
     });
 });
+
+describe("failure in calc chain", () => {
+
+    it("calc chain with no failure", () => {
+        const pc = new PabChute(new PabChuteParams(1, 0.5, 666));
+        pc.calculatedParam = pc.prms.DH;
+        const pn = new PabNombre(new PabNombreParams(666, 10, 666));
+        pn.calculatedParam = pn.prms.DH;
+        const pp = new PabPuissance(new PabPuissanceParams(666, 0.1, 0.5, 666));
+        pp.calculatedParam = pp.prms.PV;
+        Session.getInstance().clear();
+        Session.getInstance().registerNubs([ pc, pn, pp ]);
+        pn.prms.DHT.defineReference(pc, "DH");
+        pp.prms.DH.defineReference(pn, "DH");
+        const res = pp.CalcSerie();
+        expect(res.globalLog.messages.length).toBe(0);
+        expect(res.resultElement.log.messages.length).toBe(0);
+    });
+
+    it("calc chain with a failure", () => {
+        const pc = new PabChute(new PabChuteParams(1, 0.5, 666));
+        pc.calculatedParam = pc.prms.DH;
+        const pn = new PabNombre(new PabNombreParams(666, 10, 666));
+        pn.calculatedParam = pn.prms.DH;
+        const pp = new PabPuissance(new PabPuissanceParams(666, 0.1, 0.5, 666));
+        pp.calculatedParam = pp.prms.PV;
+        Session.getInstance().clear();
+        Session.getInstance().registerNubs([ pc, pn, pp ]);
+        pn.prms.DHT.defineReference(pc, "DH");
+        pp.prms.DH.defineReference(pn, "DH");
+        pc.prms.Z1.singleValue = 0.1; // Z1 < Z2 : error
+        const res = pp.CalcSerie();
+        expect(res.globalLog.messages.length).toBe(1);
+        expect(res.globalLog.messages[0].code).toBe(MessageCode.ERROR_IN_CALC_CHAIN);
+    });
+
+    it("calc chain with variation, some failures", () => {
+        const pc = new PabChute(new PabChuteParams(1, 0.5, 666));
+        pc.calculatedParam = pc.prms.DH;
+        const pn = new PabNombre(new PabNombreParams(666, 10, 666));
+        pn.calculatedParam = pn.prms.DH;
+        const pp = new PabPuissance(new PabPuissanceParams(666, 0.1, 0.5, 666));
+        pp.calculatedParam = pp.prms.PV;
+        Session.getInstance().clear();
+        Session.getInstance().registerNubs([ pc, pn, pp ]);
+        pn.prms.DHT.defineReference(pc, "DH");
+        pp.prms.DH.defineReference(pn, "DH");
+        pc.prms.Z1.singleValue = 1;
+        pc.prms.Z2.setValues(0.4, 1.9, 0.5); // 2 first steps succeed, 2 last steps fail (Z1 < Z2)
+        const res = pp.CalcSerie();
+        expect(res.globalLog.messages.length).toBe(1);
+        expect(res.globalLog.messages[0].code).toBe(MessageCode.WARNING_ERROR_IN_CALC_CHAIN_STEPS);
+    });
+
+    it("calc chain with variation, only failures", () => {
+        const pc = new PabChute(new PabChuteParams(1, 0.5, 666));
+        pc.calculatedParam = pc.prms.DH;
+        const pn = new PabNombre(new PabNombreParams(666, 10, 666));
+        pn.calculatedParam = pn.prms.DH;
+        const pp = new PabPuissance(new PabPuissanceParams(666, 0.1, 0.5, 666));
+        pp.calculatedParam = pp.prms.PV;
+        Session.getInstance().clear();
+        Session.getInstance().registerNubs([ pc, pn, pp ]);
+        pn.prms.DHT.defineReference(pc, "DH");
+        pp.prms.DH.defineReference(pn, "DH");
+        pc.prms.Z1.singleValue = 1;
+        pc.prms.Z2.setValues(1.4, 2.9, 0.5);
+        const res = pp.CalcSerie();
+        expect(res.globalLog.messages.length).toBe(1);
+        expect(res.globalLog.messages[0].code).toBe(MessageCode.ERROR_IN_CALC_CHAIN);
+    });
+
+});
diff --git a/src/nub.ts b/src/nub.ts
index e644b1af..07ff8a76 100644
--- a/src/nub.ts
+++ b/src/nub.ts
@@ -367,18 +367,44 @@ export abstract class Nub extends ComputeNode implements IObservable {
     /**
      * Calculates required Nubs so that all input data is available;
      * uses 50% of the progress
+     * @returns true if everything went OK, false otherwise
      */
-    public triggerChainCalculation() {
+    public triggerChainCalculation(): { ok: boolean, message: Message } {
         const requiredNubs1stLevel = this.getRequiredNubs();
         if (requiredNubs1stLevel.length > 0) {
             const progressStep = Nub.progressPercentageAccordedToChainCalculation / requiredNubs1stLevel.length;
             for (const rn of requiredNubs1stLevel) {
-                rn.CalcSerie();
+                const r = rn.CalcSerie();
+                if (r.hasGlobalError() || r.hasOnlyErrors) {
+                    // something has failed in chain
+                    return {
+                        ok: false,
+                        message: new Message(MessageCode.ERROR_IN_CALC_CHAIN)
+                    };
+                } else if (
+                    this.resultHasMultipleValues
+                    && (
+                        r.hasErrorMessages() // some steps failed
+                        // or upstream Nub has already triggered a warning message; pass it on
+                        || r.globalLog.contains(MessageCode.WARNING_ERROR_IN_CALC_CHAIN_STEPS)
+                    )
+                ) {
+                    // if a parameter varies, errors might have occurred for
+                    // certain steps (but not all steps)
+                    return {
+                        ok: true,
+                        message: new Message(MessageCode.WARNING_ERROR_IN_CALC_CHAIN_STEPS)
+                    };
+                }
                 this.progress += progressStep;
             }
             // round progress to accorded percentage
             this.progress = Nub.progressPercentageAccordedToChainCalculation;
         }
+        return {
+            ok: true,
+            message: undefined
+        };
     }
 
     /**
@@ -392,9 +418,21 @@ export abstract class Nub extends ComputeNode implements IObservable {
         const variated: Array<{ param: ParamDefinition, values: ParamValues }> = [];
 
         // prepare calculation
+        let extraLogMessage: Message; // potential chain calculation warning to add to result at the end
         this.progress = 0;
         this.resetResult();
-        this.triggerChainCalculation();
+        const ccRes = this.triggerChainCalculation();
+        if (ccRes.ok) {
+            // might still have a warning log
+            if (ccRes.message !== undefined) {
+                extraLogMessage = ccRes.message;
+            }
+        } else {
+            // something went wrong in the chain
+            this._result = new Result(undefined, this);
+            this._result.globalLog.add(ccRes.message);
+            return this._result;
+        }
         this.copySingleValuesToSandboxValues();
 
         // check which values are variating, if any
@@ -492,10 +530,14 @@ export abstract class Nub extends ComputeNode implements IObservable {
             }
             // String()ify numbers below, to avoid decimals formatting on screen (ex: "3.000 errors encoutered...")
             if (errors > 0) {
-                this._result.globalLog.add(new Message(MessageCode.ERROR_ABSTRACT, { nb: String(errors) }));
+                this._result.globalLog.add(
+                    new Message(MessageCode.WARNING_ERRORS_ABSTRACT, { nb: String(errors) })
+                );
             }
             if (warnings > 0) {
-                this._result.globalLog.add(new Message(MessageCode.WARNING_ABSTRACT, { nb: String(warnings) }));
+                this._result.globalLog.add(
+                    new Message(MessageCode.WARNING_WARNINGS_ABSTRACT, { nb: String(warnings) })
+                );
             }
         }
 
@@ -506,6 +548,10 @@ export abstract class Nub extends ComputeNode implements IObservable {
 
         this.notifyResultUpdated();
 
+        if (extraLogMessage !== undefined) {
+            this._result.globalLog.add(extraLogMessage);
+        }
+
         return this._result;
     }
 
diff --git a/src/session.ts b/src/session.ts
index 9bb601ea..50cb9f90 100644
--- a/src/session.ts
+++ b/src/session.ts
@@ -151,6 +151,15 @@ export class Session {
         this._nubs.push(n);
     }
 
+    /**
+     * Adds many existing Nubs to the session
+     */
+    public registerNubs(nubs: Nub[]) {
+        for (const n of nubs) {
+            this.registerNub(n);
+        }
+    }
+
     /**
      * Removes all Nubs from the Session
      */
diff --git a/src/util/log.ts b/src/util/log.ts
index 67cd8b4a..35eb67d3 100644
--- a/src/util/log.ts
+++ b/src/util/log.ts
@@ -1,4 +1,4 @@
-import { Message } from "./message";
+import { Message, MessageCode } from "./message";
 
 // tslint:disable-next-line:class-name
 export class cLog {
@@ -43,4 +43,17 @@ export class cLog {
     public toString(): string {
         return this._messages.join("\n");
     }
+
+    /**
+     * @param mc message code you're looking for
+     * @returns true if log contains at least one occurrence of given message code
+     */
+    public contains(mc: MessageCode): boolean {
+        for (const m of this.messages) {
+            if (m.code === mc) {
+                return true;
+            }
+        }
+        return false;
+    }
 }
diff --git a/src/util/message.ts b/src/util/message.ts
index 3c128937..4495f7b1 100644
--- a/src/util/message.ts
+++ b/src/util/message.ts
@@ -5,7 +5,7 @@ export enum MessageCode {
     ERROR_OK,
 
     /** abstract showing number of error messages encountered in an iterative calculation */
-    ERROR_ABSTRACT,
+    WARNING_ERRORS_ABSTRACT,
 
     /** calculation of Z1 in Fluvial regime has failed (upstream abscissa not present in results) */
     ERROR_BIEF_Z1_CALC_FAILED,
@@ -57,6 +57,16 @@ export enum MessageCode {
      */
     ERROR_ELEVATION_ZI_LOWER_THAN_Z2,
 
+    /**
+     * Something failed when calculating upstream Nubs
+     */
+    ERROR_IN_CALC_CHAIN,
+
+    /**
+     * Something failed in certain steps (but not all), when calculating upstream Nubs with varying parameter
+     */
+    WARNING_ERROR_IN_CALC_CHAIN_STEPS,
+
     /**
      * les bornes de l'intervalle d'un ParamDomain sont incorrectes
      */
@@ -288,7 +298,7 @@ export enum MessageCode {
     ERROR_STRUCTURE_Z_EGAUX_Q_NON_NUL,
 
     /** abstract showing number of warning messages encountered in an iterative calculation */
-    WARNING_ABSTRACT,
+    WARNING_WARNINGS_ABSTRACT,
 
     /** La cote de fond aval est plus élevée que la code de l'eau aval */
     WARNING_DOWNSTREAM_BOTTOM_HIGHER_THAN_WATER,
diff --git a/src/util/result.ts b/src/util/result.ts
index 83db9cbb..773328b3 100644
--- a/src/util/result.ts
+++ b/src/util/result.ts
@@ -172,16 +172,26 @@ export class Result extends JalhydObject {
     }
 
     /**
-     * @returns true if at least one of the log messages (general log or result elements logs)
-     *          has an error-level message, i.e. true if at least one calculation has failed
+     * @returns true if globalLog has at least one message with level ERROR
      */
-    public hasErrorMessages(): boolean {
-        // global log
+    public hasGlobalError(): boolean {
         for (const m of this._globalLog.messages) {
             if (m.getSeverity() === MessageSeverity.ERROR) {
                 return true;
             }
         }
+        return false;
+    }
+
+    /**
+     * @returns true if at least one of the log messages (general log or result elements logs)
+     *          has an error-level message, i.e. true if at least one calculation has failed
+     */
+    public hasErrorMessages(): boolean {
+        // global log
+        if (this.hasGlobalError()) {
+            return true;
+        }
         // result elements logs
         for (const r of this._resultElements) {
             if (r.hasErrorMessages()) {
-- 
GitLab