Morendil
commented
5 years ago One thing that makes this hard to investigate is that it manifests in many ways, some of which can be seen on a small scale (as in the test above) and some of which have to be embraced at a larger scale. I've created a gist with the trace of a calculation based on a version of Core that removes cycle checks altogether. This trace is from executing this Mes Aides test. It's 10K lines long - that's how deep the stack reaches.
One of the things that's going in there is that we calculate salaire_imposable 6 times for different periods, to calculate autonomie_financiere. If you search for autonomie_financiere in the trace it quickly becomes apparent that we compute it at ever increasing depths of stack, on each occasion for a period reaching further and further back in time.
The hunch I'm investigating is that this isn't about detecting "cycles" at all. It seems clear for this particular observation that the problem can be also framed as why are we even attempting to compute salaire_imposable in the first place, when we don't have a salaire_de_base in the first place ? This is a semantic determination, not a purely syntactic one. I, a human, know that it's pointless to try and compute a taxable salary when there is no salary information in the situation we are computing.
Is there a clue we could follow ? It seems like there is:
class salaire_de_base(Variable):
value_type = float
entity = Individu
label = u"Salaire de base, en général appelé salaire brut, la 1ère ligne sur la fiche de paie"
set_input = set_input_divide_by_period
reference = u'http://www.insee.fr/fr/methodes/default.asp?page=definitions/salaire-mensuel-base-smb.htm'
definition_period = MONTHWell, it doesn't have a default value! That makes sense, because there's really no "default" base salary in salary computation - but there can be the absence of a salary. And in fact we can short-circuit a bunch of calculations if we could detect this. Unfortunately, the code for variables as written has a notion of a "default default value", and so we don't take advantage of this clue.
However, this doesn't save us yet. I've tried forcing Core to skip the computation if the variable being computed was salaire_imposable - and we still hit a cycle - but a different one, which is a clue that cycles in fact abound in the France model. (A good hint that we should solve this problem in a general way and not piecemeal, I think.)
Here is the resulting trace. This looks more like a "true" circular definition: we have the eligibility for AAH computed based on a bunch of income sources which include another aid called ASI/ASPA, for which eligibility in turn hinges on a bunch of income sources including AAH itself.
However, it's important to note that it's not a real cycle in the sense that each time around we shift the period of evaluation towards the past. It would be reasonable to compute a situation in which we want to know, for instance, what fluctuations exist over time as a result of being granted aids in the past which causes someone's income to go over a threshold which causes them not to be eligible for this aid.
What doesn't really make sense in the context of the test is this: we are evaluating eligibility for ASI/ASPA at infinite depths for a situation where its age condition is not fulfilled. Again we have a semantic understanding that we could short-circuit a computation - this time not because a variable is absent but because a variable is present.
This is not a new issue and is linked to vectorization of the computations. In short, we've assumed so far that vector computation necessarily means eager evaluation. We are evaluating ASI/ASPA for a 38-year-old on the chance that the array might contain elderly people. We've tried to improve this with the max_nb_cycles parameter, but this has a huge problem: max_nb_cycles only stops a computation when a cycle is detected, which means that we have been at least once through the loop of unnecessary computations.
This is absurd in the singular case, but it also makes kind of an absurdity, unless I'm very mistaken about the execution behaviour, of our performance story for vector computations. The bulk of the population is under 65, so just how much performance we're throwing away even in a vector context to run one loop of superfluous evaluations for everyone, relative to what we'd be throwing away by performing the computation sequentially, is an open question.
The good news is that we don't have to choose between the two evils of sequential computation or vast amounts of unnecessary computation, at least I don't believe we do. There seem to be technical solutions (lazyarray, cytoolz), or in the worst case we could syntactically elevate eligibility conditions to a special status, perform them always before computing income bases (or other expensive aggregates), and filter the arrays post-eligibility to perform the income basis computation only on entities that pass the first level of eligibility.
But anyway, the TLDR here is: I'd like to reframe this problem from detecting cycles to short-circuiting computation.
bonjourmauko
benjello
fpagnoux
Minimal repro:
The results of computing
assandrfrshould not depend on the order of evaluation, but they do:This is impacting France and indirectly Mes-Aides (preventing an upgrade to the latest version of France in production). The France model is very ramified and cycles abound; on inspection I think the current solution (
max_nb_cycles) is misguided.Connected to openfisca/openfisca-france#1211