-
Notifications
You must be signed in to change notification settings - Fork 75
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Make explicit input variables temporal inferences #460
Comments
Some comments:
I hope this helps and keep on the good work ! |
This could be interesting, but is out of the scope of this specific issue on
This could be interesting, but is out of the scope of this specific issue on
Do you have an example of a computing that would be impossible to do without such an inference? If not, a case where it would be very cumbersome? |
I disagree on one point: For your last remark, I think of the obvious My suggestion was to recognize that the need of extrapolating a value to the past or future without having it permanent may be a good option to replace
|
Very clear, thank you! I updated #458 accordingly 🙂 |
I think we'll want to look at this again in the context of #749 and openfisca/openfisca-france#1211 In Core 24.9.8 and running the YAML tests for France 32.0.1, I disabled the following two lines (commenting them out): openfisca-core/openfisca_core/simulations.py Lines 174 to 175 in 400d82f
This results in 21 failures, listed below. The France model as of 32.0.1 has 18 variables with a base_function explicitly assigned:
However, the reliance on base_function is much broader, as variables of types other than int and float are assigned the base function
|
This issue handles a part of #458.
For
base_function
, the issue rose with the formularsa
, where macrosimulation (where missing values should be the default ones (or raise warnings, @benjello?) and production (where missing values should be inferred) expected behaviours differ.Default values in input variables are the source of the issue: if a formula needs values that are undefined (e.g. because they are in previous months from the calculation), it won't crash nor log anything because the input variables return their default values. This inference behaviour seems to be fine for macrosimulation, but production would expect a time-based inference, where the value is copied from its closest defined value rather than a global default one. Changing the inference mode means changing the
base_function
, but that raises conflicts between uses.There is a consensus between @cbenz @michelbl @fpagnoux that
base_function
is incomprehensible and counterintuitive. It is at least misnamed and it should be more explicit that it is currently used purely as an extrapolation mechanism.There is also a consensus that defaulting to the simulation period when requesting to compute a formula is not a proper behaviour: this piece of information should be mandatory to avoid incomprehensible behaviour on omission.
Discussed possible solutions
Candidates
Eliminated
Suggested action plan
base_function
, check how many tests fail and evaluate the amount of work to refactor the inputs.The text was updated successfully, but these errors were encountered: