From d7f140334e67a8c16cff2ac9ea367d2cc87ddf2e Mon Sep 17 00:00:00 2001 From: Maxime Quandalle Date: Sun, 22 Nov 2020 17:03:46 +0100 Subject: [PATCH] =?UTF-8?q?=F0=9F=90=8E=20Acc=C3=A9l=C3=A8re=20l'inversion?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit La fonction `uniroot` prend 2 paramètres d'amorçage "min" et "max" qui nous définissions jusqu'alors comme des minimums et maximum absolus -10^8 et +10^8. Vu que nous sommes obligés de calculer au moins une première valeur à l'extérieur de `uniroot` notamment pour calculer les variables manquantes, ce commit permet de ré-utiliser ce calcul dans l'amorçage d' `uniroot`. Les gains de performances sont détaillés dans la PR associée. Par ailleurs supprime l'option "valeurs négatives possibles" rendue obsolète. Il y a des légers décalages d'1€ sur une dizaine de snapshots qui liés à des arrondis à l'euro. On calcule en effet les inversions à 10 centimes près et on peut donc tomber sur une valeur de xx,54€ là où la vraie valeur est xx,48€ ce qui donne 1€ de différence avec l'arrondi alors que la différence initialement calculée est inférieure à 10 centimes. Par curiosité j'ai rejoué les tests de non-régressions en changeant les paramètres d'`uniroot` pour avoir une précision au centime près (en augmentant le nombre max d'itération à 50) et il se trouve que sur la dizaine de tests différents entre ce commit et la version d'avant une moitié des arrondis à l'euro étaient faux avant et corrects maintenant et inversement pour l'autre moitié. --- mon-entreprise/source/rules/dirigeant.yaml | 1 - .../__snapshots__/simulations.jest.js.snap | 24 ++-- .../mecanisms/InversionNumérique.js | 10 +- publicodes/source/mecanisms/inversion.ts | 127 ++++++++++++------ publicodes/source/nodeUnits.ts | 2 +- publicodes/source/uniroot.ts | 11 +- 6 files changed, 112 insertions(+), 63 deletions(-) diff --git a/mon-entreprise/source/rules/dirigeant.yaml b/mon-entreprise/source/rules/dirigeant.yaml index cfbccdf26..94785e95c 100644 --- a/mon-entreprise/source/rules/dirigeant.yaml +++ b/mon-entreprise/source/rules/dirigeant.yaml @@ -580,7 +580,6 @@ dirigeant . indépendant . revenu professionnel: - revenu net après impôt - entreprise . chiffre d'affaires - entreprise . chiffre d'affaires minimum - valeurs négatives possibles: oui arrondi: oui dirigeant . indépendant . assiette des cotisations: diff --git a/mon-entreprise/test/regressions/__snapshots__/simulations.jest.js.snap b/mon-entreprise/test/regressions/__snapshots__/simulations.jest.js.snap index 351a9f896..fd52c2ab3 100644 --- a/mon-entreprise/test/regressions/__snapshots__/simulations.jest.js.snap +++ b/mon-entreprise/test/regressions/__snapshots__/simulations.jest.js.snap @@ -34,7 +34,7 @@ exports[`calculate aide-déclaration-indépendant: conjoint collaborateur 5`] = exports[`calculate aide-déclaration-indépendant: débit de tabac 1`] = `"[50000,3177,5672,101,8950,41050]"`; -exports[`calculate aide-déclaration-indépendant: international 1`] = `"[50000,0,14610,101,14711,35289]"`; +exports[`calculate aide-déclaration-indépendant: international 1`] = `"[50000,0,14612,101,14713,35287]"`; exports[`calculate aide-déclaration-indépendant: international 2`] = `"[50000,1267,11893,101,13261,36739]"`; @@ -160,7 +160,7 @@ exports[`calculate simulations-indépendant: inversions 6`] = `"[19000,5928,1307 exports[`calculate simulations-indépendant: inversions 7`] = `"[18000,5625,12375,12861,0,12375,2000,20000]"`; -exports[`calculate simulations-indépendant: échelle de revenus 1`] = `"[1858,1358,500,548,0,500,0,1858]"`; +exports[`calculate simulations-indépendant: échelle de revenus 1`] = `"[1859,1359,500,548,0,500,0,1859]"`; exports[`calculate simulations-indépendant: échelle de revenus 2`] = `"[2467,1467,1000,1064,0,1000,0,2467]"`; @@ -170,7 +170,7 @@ exports[`calculate simulations-indépendant: échelle de revenus 4`] = `"[3682,1 exports[`calculate simulations-indépendant: échelle de revenus 5`] = `"[7428,2428,5000,5199,0,5000,0,7428]"`; -exports[`calculate simulations-indépendant: échelle de revenus 6`] = `"[14595,4595,10000,10393,0,10000,0,14595]"`; +exports[`calculate simulations-indépendant: échelle de revenus 6`] = `"[14596,4596,10000,10394,0,10000,0,14596]"`; exports[`calculate simulations-indépendant: échelle de revenus 7`] = `"[139595,39595,100000,103788,24909,75091,0,139595]"`; @@ -196,11 +196,11 @@ exports[`calculate simulations-professions-libérales: auxiliaire médical 1`] = exports[`calculate simulations-professions-libérales: auxiliaire médical 2`] = `"[30000,0,8077,21923,932,20991]"`; -exports[`calculate simulations-professions-libérales: auxiliaire médical 3`] = `"[300000,0,61784,238217,81297,156920]"`; +exports[`calculate simulations-professions-libérales: auxiliaire médical 3`] = `"[300000,0,61784,238216,81297,156919]"`; -exports[`calculate simulations-professions-libérales: avocat 1`] = `"[50000,0,11410,38590,4753,33837]"`; +exports[`calculate simulations-professions-libérales: avocat 1`] = `"[50000,0,11410,38589,4753,33836]"`; -exports[`calculate simulations-professions-libérales: avocat 2`] = `"[50000,0,11770,38229,4711,33518]"`; +exports[`calculate simulations-professions-libérales: avocat 2`] = `"[50000,0,11770,38230,4711,33519]"`; exports[`calculate simulations-professions-libérales: expert-comptable 1`] = `"[20000,0,5042,14958,0,14958]"`; @@ -210,7 +210,7 @@ exports[`calculate simulations-professions-libérales: médecin 1`] = `"[50000,0 exports[`calculate simulations-professions-libérales: médecin 2`] = `"[50000,0,20229,29771,2334,27437]"`; -exports[`calculate simulations-professions-libérales: médecin 3`] = `"[300000,0,86481,213520,73147,140373]"`; +exports[`calculate simulations-professions-libérales: médecin 3`] = `"[300000,0,86481,213519,73147,140372]"`; exports[`calculate simulations-professions-libérales: médecin 4`] = `"[400000,0,106201,293799,115768,178031]"`; @@ -254,7 +254,7 @@ Notifications affichées : dirigeant . assimilé salarié . réduction ACRE . no `; exports[`calculate simulations-rémunération-dirigeant (assimilé salarié): ACRE 3`] = ` -"[1893,0,0,21948,4,40] +"[1893,0,0,21947,4,40] Notifications affichées : dirigeant . assimilé salarié . réduction ACRE . notification taux annuel" `; @@ -286,9 +286,9 @@ exports[`calculate simulations-rémunération-dirigeant (assimilé salarié): av exports[`calculate simulations-rémunération-dirigeant (assimilé salarié): échelle de rémunération 1`] = `"[0,0,0,0,0,0]"`; -exports[`calculate simulations-rémunération-dirigeant (assimilé salarié): échelle de rémunération 2`] = `"[14,0,0,140,0,1]"`; +exports[`calculate simulations-rémunération-dirigeant (assimilé salarié): échelle de rémunération 2`] = `"[14,0,0,139,0,1]"`; -exports[`calculate simulations-rémunération-dirigeant (assimilé salarié): échelle de rémunération 3`] = `"[62,0,0,323,0,2]"`; +exports[`calculate simulations-rémunération-dirigeant (assimilé salarié): échelle de rémunération 3`] = `"[62,0,0,324,0,2]"`; exports[`calculate simulations-rémunération-dirigeant (assimilé salarié): échelle de rémunération 4`] = `"[204,0,0,2591,2,5]"`; @@ -298,7 +298,7 @@ exports[`calculate simulations-rémunération-dirigeant (assimilé salarié): é exports[`calculate simulations-rémunération-dirigeant (assimilé salarié): échelle de rémunération 7`] = `"[2341,0,0,27089,4,46]"`; -exports[`calculate simulations-rémunération-dirigeant (assimilé salarié): échelle de rémunération 8`] = `"[4758,0,0,52757,4,46]"`; +exports[`calculate simulations-rémunération-dirigeant (assimilé salarié): échelle de rémunération 8`] = `"[4758,0,0,52758,4,46]"`; exports[`calculate simulations-rémunération-dirigeant (auto-entrepreneur): ACRE 1`] = ` "[0,0,806,2046,2,0] @@ -603,7 +603,7 @@ exports[`calculate simulations-salarié: frais pro - IKV 1`] = `"[4367,0,3200,25 exports[`calculate simulations-salarié: frais pro - IKV 2`] = `"[4346,0,3200,2511,2314]"`; -exports[`calculate simulations-salarié: frais pro - IKV 3`] = `"[2764,0,2152,1681,1630]"`; +exports[`calculate simulations-salarié: frais pro - IKV 3`] = `"[2764,0,2151,1681,1630]"`; exports[`calculate simulations-salarié: frais pro - titres restaurant 1`] = `"[2519,0,2000,1521,1487]"`; diff --git a/publicodes/source/components/mecanisms/InversionNumérique.js b/publicodes/source/components/mecanisms/InversionNumérique.js index f4987c6be..2658fbb00 100644 --- a/publicodes/source/components/mecanisms/InversionNumérique.js +++ b/publicodes/source/components/mecanisms/InversionNumérique.js @@ -6,27 +6,25 @@ export default function InversionNumérique({ nodeValue, explanation }) { {explanation.inversionFailed ? ( <> - {' '}

Cette valeur devrait pouvoir être estimée à partir d'une autre variable qui possède une formule de calcul et dont la valeur a été fixée dans la simulation :

- {makeJsx(explanation.inversedWith)} + {makeJsx(explanation.inversionGoal)}

Malheureusement, il a été impossible de retrouver une valeur pour - cette formule qui permette d'atterir sur la valeur demandée. + cette formule qui permette d'atterrir sur la valeur demandée.

- ) : explanation.inversedWith ? ( + ) : explanation.inversionGoal ? ( <> - {' '}

Cette valeur a été estimée à partir d'une autre variable qui possède une formule de calcul et dont la valeur a été fixée dans la simulation :

- {makeJsx(explanation.inversedWith)} + {makeJsx(explanation.inversionGoal)} ) : ( <> diff --git a/publicodes/source/mecanisms/inversion.ts b/publicodes/source/mecanisms/inversion.ts index f64c74adc..6225c3e44 100644 --- a/publicodes/source/mecanisms/inversion.ts +++ b/publicodes/source/mecanisms/inversion.ts @@ -1,16 +1,24 @@ import { evaluationFunction } from '..' import InversionNumérique from '../components/mecanisms/InversionNumérique' -import { registerEvaluationFunction } from '../evaluation' +import { mergeMissing, registerEvaluationFunction } from '../evaluation' import { convertNodeToUnit } from '../nodeUnits' import uniroot from '../uniroot' import { parseUnit } from '../units' +// The user of the inversion mechanism has to define a list of "inversion +// candidates". At runtime, the evaluation function of the mechanism will look +// at the situation value of these candidates, and use the first one that is +// defined as its "goal" for the inversion +// +// The game is then to find an input such as the computed value of the "goal" is +// equal to its situation value, mathematically we search for the zero of the +// function x → f(x) - goal. The iteration logic between each test is +// implemented in the `uniroot` file. export const evaluateInversion: evaluationFunction = function(node) { - // TODO : take applicability into account here - let inversedWith = node.explanation.inversionCandidates.find( - n => this.parsedSituation[n.dottedName] != undefined + let inversionGoal = node.explanation.inversionCandidates.find( + candidate => this.parsedSituation[candidate.dottedName] != undefined ) - if (!inversedWith) { + if (!inversionGoal) { return { ...node, missingVariables: { @@ -22,49 +30,85 @@ export const evaluateInversion: evaluationFunction = function(node) { nodeValue: null } } - inversedWith = this.evaluateNode(inversedWith) + inversionGoal = this.evaluateNode(inversionGoal) + const unit = this.parsedRules[node.explanation.ruleToInverse].unit + const originalCache = { ...this.cache } const originalSituation = { ...this.parsedSituation } + + let inversionNumberOfIterations = 0 const evaluateWithValue = (n: number) => { + inversionNumberOfIterations++ this.cache = { _meta: { ...originalCache._meta } } this.parsedSituation = { ...originalSituation, - [inversedWith.dottedName]: undefined, + [inversionGoal.dottedName]: undefined, [node.explanation.ruleToInverse]: { nodeValue: n, - unit: this.parsedRules[node.explanation.ruleToInverse].unit + unit } } - return this.evaluateNode(inversedWith) + return convertNodeToUnit(unit, this.evaluateNode(inversionGoal)) } - // si fx renvoie null pour une valeur numérique standard, disons 2000, on peut - // considérer que l'inversion est impossible du fait de variables manquantes - // TODO fx peut être null pour certains x, et valide pour d'autres : on peut implémenter ici le court-circuit - const randomAttempt = evaluateWithValue(2000) - const nodeValue = - randomAttempt.nodeValue === null - ? null - : // cette fonction détermine l'inverse d'une fonction sans faire trop d'itérations - uniroot( - x => { - const candidateNode = evaluateWithValue(x) - return ( - candidateNode.nodeValue - - // TODO: convertNodeToUnit migth return null or false - (convertNodeToUnit(candidateNode.unit, inversedWith) - .nodeValue as number) - ) - }, - node.explanation.negativeValuesAllowed ? -1000000 : 0, - 100000000, - 0.1, - 10, - 1 - ) + const goal = convertNodeToUnit(unit, inversionGoal).nodeValue as number + let nodeValue: number | null | undefined = null + + // We do some blind attempts here to avoid using the default minimum and + // maximum of +/- 10^8 that are required by the `uniroot` function. For the + // first attempt we use the goal value as a very rough first approximation. + // For the second attempt we do a proportionality coefficient with the result + // from the first try and the goal value. The two attempts are then used in + // the following way: + // - if both results are `null` we assume that the inversion is impossible + // because of missing variables + // - otherwise, we calculate the missing variables of the node as the union of + // the missings variables of our two attempts + // - we cache the result of our two attempts so that `uniroot` doesn't + // recompute them + const x1 = goal + const y1Node = evaluateWithValue(x1) + const y1 = y1Node.nodeValue as number + const coeff = y1 > goal ? 0.9 : 1.2 + const x2 = y1 !== null ? (x1 * goal * coeff) / y1 : 2000 + const y2Node = evaluateWithValue(x2) + const y2 = y2Node.nodeValue as number + + const missingVariables = mergeMissing( + y1Node.missingVariables, + y2Node.missingVariables + ) + + if (y1 !== null || y2 !== null) { + // The `uniroot` function parameter. It will be called with its `min` and + // `max` arguments, so we can use our cached nodes if the function is called + // with the already computed x1 or x2. + const test = (x: number): number => { + const y = x === x1 ? y1 : x === x2 ? y2 : evaluateWithValue(x).nodeValue + return (y as number) - goal + } + + const defaultMin = -1000000 + const defaultMax = 100000000 + const nearestBelowGoal = + y2 !== null && y2 < goal && (y2 > y1 || y1 > goal) + ? x2 + : y1 !== null && y1 < goal && (y1 > y2 || y2 > goal) + ? x1 + : defaultMin + const nearestAboveGoal = + y2 !== null && y2 > goal && (y2 < y1 || y1 < goal) + ? x2 + : y1 !== null && y1 > goal && (y1 < y2 || y2 < goal) + ? x1 + : defaultMax + + nodeValue = uniroot(test, nearestBelowGoal, nearestAboveGoal, 0.1, 10, 1) + } if (nodeValue === undefined) { + nodeValue = null originalCache._meta.inversionFail = true } else { // For performance reason, we transfer the inversion cache @@ -72,17 +116,23 @@ export const evaluateInversion: evaluationFunction = function(node) { originalCache[k] = value }) } + + // // Uncomment to display the two attempts and their result + // console.table([{ x: x1, y: y1 }, { x: x2, y: y2 }]) + // console.log('iteration:', inversionNumberOfIterations) + this.cache = originalCache this.parsedSituation = originalSituation + return { ...node, - nodeValue: nodeValue ?? null, + nodeValue, explanation: { ...node.explanation, - inversionFail: nodeValue === undefined, - inversedWith + inversionGoal, + inversionNumberOfIterations }, - missingVariables: randomAttempt.missingVariables + missingVariables } } @@ -96,8 +146,7 @@ export const mecanismInversion = (recurse, v, dottedName) => { unit: v.unité && parseUnit(v.unité), explanation: { ruleToInverse: dottedName, - inversionCandidates: v.avec.map(recurse), - negativeValuesAllowed: v['valeurs négatives possibles'] === 'oui' + inversionCandidates: v.avec.map(recurse) }, jsx: InversionNumérique, category: 'mecanism', diff --git a/publicodes/source/nodeUnits.ts b/publicodes/source/nodeUnits.ts index 00a35e344..50d82032c 100644 --- a/publicodes/source/nodeUnits.ts +++ b/publicodes/source/nodeUnits.ts @@ -12,7 +12,7 @@ export function simplifyNodeUnit(node) { } export function convertNodeToUnit( - to: Unit, + to: Unit | undefined, node: EvaluatedNode ) { const temporalValue = diff --git a/publicodes/source/uniroot.ts b/publicodes/source/uniroot.ts index da4da1198..3023d7f86 100644 --- a/publicodes/source/uniroot.ts +++ b/publicodes/source/uniroot.ts @@ -45,7 +45,8 @@ export default function uniroot( newStep: number, // Step at this iteration prevStep: number, // Distance from the last but one to the last approximation p: number, // Interpolation step is calculated in the form p/q; division is delayed until the last moment - q: number + q: number, + fallback: number | undefined = undefined while (maxIter-- > 0) { prevStep = b - a @@ -108,8 +109,10 @@ export default function uniroot( if ((fb > 0 && fc > 0) || (fb < 0 && fc < 0)) { ;(c = a), (fc = fa) // Adjust c for it to have a sign opposite to that of b } + + if (Math.abs(fb) < acceptableErrorTol) { + fallback = b + } } - if (Math.abs(fb) < acceptableErrorTol) { - return b - } + return fallback }