Skip to content
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

Remove extra_params altogether from Core #847

Merged
merged 2 commits into from
Mar 21, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 13 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,18 @@
# Changelog

# 28.0.0 [#847](https://github.com/openfisca/openfisca-core/pull/847)

#### Breaking changes

Remove extra_params altogether from Core. This puts all formulas on an equal footing and simplifies
simulation and data storage code.

#### Migration notes

For almost all users, no migration is needed.

You may be impacted only if you were computing (for instance in a notebook) one of the two variables from France that used this non-standard extension to the `calculate(variable, period)` API: `rsa_fictif` or `ppa_fictive`. In that case you had to supply the additional parameter `mois_demande`. Use `rsa` or `ppa` directly, and go back to the standard API.

### 27.0.2 [#844](https://github.com/openfisca/openfisca-core/pull/844)

> Note: Versions `27.0.0` and `27.0.1` have been unpublished as the former accidentally introduced a bug affecting test inputs. Please use version `27.0.2` or more recent.
Expand Down
14 changes: 7 additions & 7 deletions openfisca_core/base_functions.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
"""


def requested_period_default_value(holder, period, *extra_params):
def requested_period_default_value(holder, period):
"""
This formula is used for variables for which we don't want to make any inference about the value for a given period based on past or future values.

Expand All @@ -17,7 +17,7 @@ def requested_period_default_value(holder, period, *extra_params):
return None


def requested_period_last_value(holder, period, *extra_params, **kwargs):
def requested_period_last_value(holder, period, **kwargs):
"""
This formula is used for variables that are constants between events and period size independent.
If the variable has no formula, it will return the latest known value of the variable
Expand All @@ -30,20 +30,20 @@ def requested_period_last_value(holder, period, *extra_params, **kwargs):
known_periods = sorted(known_periods, key=lambda period: period.start, reverse = True)
for last_period in known_periods:
if last_period.start <= period.start:
return holder.get_array(last_period, extra_params)
return holder.get_array(last_period)
if accept_future_value:
next_period = known_periods[-1]
return holder.get_array(next_period, extra_params)
return holder.get_array(next_period)
return None


def requested_period_last_or_next_value(holder, period, *extra_params):
def requested_period_last_or_next_value(holder, period):
"""
This formula is used for variables that are constants between events and period size independent.
If the variable has no formula, it will return the latest known value of the variable, or the next value if there is no past value.
"""
return requested_period_last_value(holder, period, *extra_params, accept_future_value = True)
return requested_period_last_value(holder, period, accept_future_value = True)


def missing_value(holder, period, *extra_params):
def missing_value(holder, period):
raise ValueError("Missing value for variable {} at {}".format(holder.variable.name, period))
44 changes: 8 additions & 36 deletions openfisca_core/data_storage.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,31 +19,24 @@ def __init__(self, is_eternal = False):
self._arrays = {}
self.is_eternal = is_eternal

def get(self, period, extra_params = None):
def get(self, period):
if self.is_eternal:
period = periods.period(ETERNITY)
period = periods.period(period)

values = self._arrays.get(period)
if values is None:
return None
if extra_params:
return values.get(tuple(extra_params))
if isinstance(values, dict):
return next(iter(values.values()))
return values

def put(self, value, period, extra_params = None):
def put(self, value, period):
if self.is_eternal:
period = periods.period(ETERNITY)
period = periods.period(period)

if not extra_params:
self._arrays[period] = value
else:
if self._arrays.get(period) is None:
self._arrays[period] = {}
self._arrays[period][tuple(extra_params)] = value
self._arrays[period] = value

def delete(self, period = None):
if period is None:
Expand Down Expand Up @@ -105,45 +98,30 @@ def _decode_file(self, file):
else:
return np.load(file)

def get(self, period, extra_params = None):
def get(self, period):
if self.is_eternal:
period = periods.period(ETERNITY)
period = periods.period(period)

values = self._files.get(period)
if values is None:
return None
if extra_params:
extra_params = tuple(str(param) for param in extra_params)
value = values.get(extra_params)
if value is None:
return None
return self._decode_file(value)
if isinstance(values, dict):
return self._decode_file(next(iter(values.values())))
return self._decode_file(values)

def put(self, value, period, extra_params = None):
def put(self, value, period):
if self.is_eternal:
period = periods.period(ETERNITY)
period = periods.period(period)

filename = str(period)
if extra_params:
extra_params = tuple(str(param) for param in extra_params)
filename = '{}_{}'.format(
filename, '_'.join(extra_params))
path = os.path.join(self.storage_dir, filename) + '.npy'
if isinstance(value, EnumArray):
self._enums[path] = value.possible_values
value = value.view(np.ndarray)
np.save(path, value)
if not extra_params:
self._files[period] = path
else:
if self._files.get(period) is None:
self._files[period] = {}
self._files[period][extra_params] = path
self._files[period] = path

def delete(self, period = None):
if period is None:
Expand Down Expand Up @@ -172,14 +150,8 @@ def restore(self):
continue
path = os.path.join(self.storage_dir, filename)
filename_core = filename.rsplit('.', 1)[0]
if '_' in filename_core:
period, extra_params_str = filename_core.split('_', 1)
period = periods.period(period)
extra_params = tuple(extra_params_str.split('_'))
files.setdefault(period, {})[extra_params] = path
else:
period = periods.period(filename_core)
files[period] = path
period = periods.period(filename_core)
files[period] = path

def __del__(self):
if self.preserve_storage_dir:
Expand Down
22 changes: 11 additions & 11 deletions openfisca_core/holders.py
Original file line number Diff line number Diff line change
Expand Up @@ -81,19 +81,19 @@ def delete_arrays(self, period = None):
if self._disk_storage:
self._disk_storage.delete(period)

def get_array(self, period, extra_params = None):
def get_array(self, period):
"""
Get the value of the variable for the given period (and possibly a list of extra parameters).
Get the value of the variable for the given period.

If the value is not known, return ``None``.
"""
if self.variable.is_neutralized or ((self.variable.end is not None) and (period.start.date > self.variable.end)):
if self.variable.is_neutralized:
return self.default_array()
value = self._memory_storage.get(period, extra_params)
value = self._memory_storage.get(period)
if value is not None:
return value
if self._disk_storage:
return self._disk_storage.get(period, extra_params)
return self._disk_storage.get(period)

def get_memory_usage(self):
"""
Expand Down Expand Up @@ -207,7 +207,7 @@ def _to_array(self, value):
.format(value, self.variable.name, self.variable.dtype, value.dtype))
return value

def _set(self, period, value, extra_params = None):
def _set(self, period, value):
value = self._to_array(value)
if self.variable.definition_period != ETERNITY:
if period is None:
Expand All @@ -234,16 +234,16 @@ def _set(self, period, value, extra_params = None):

should_store_on_disk = (
self._on_disk_storable and
self._memory_storage.get(period, extra_params) is None and # If there is already a value in memory, replace it and don't put a new value in the disk storage
self._memory_storage.get(period) is None and # If there is already a value in memory, replace it and don't put a new value in the disk storage
psutil.virtual_memory().percent >= self.simulation.memory_config.max_memory_occupation_pc
)

if should_store_on_disk:
self._disk_storage.put(value, period, extra_params)
self._disk_storage.put(value, period)
else:
self._memory_storage.put(value, period, extra_params)
self._memory_storage.put(value, period)

def put_in_cache(self, value, period, extra_params = None):
def put_in_cache(self, value, period):
if self._do_not_store:
return

Expand All @@ -252,7 +252,7 @@ def put_in_cache(self, value, period, extra_params = None):
self.variable.name in self.simulation.tax_benefit_system.cache_blacklist):
return

self._set(period, value, extra_params)
self._set(period, value)

def default_array(self):
"""
Expand Down
16 changes: 7 additions & 9 deletions openfisca_core/simulations.py
Original file line number Diff line number Diff line change
Expand Up @@ -139,10 +139,8 @@ def calculate(self, variable_name, period, **parameters):

self._check_period_consistency(period, variable)

extra_params = parameters.get('extra_params', ())

# First look for a value already cached
cached_array = holder.get_array(period, extra_params)
cached_array = holder.get_array(period)
if cached_array is not None:
if self.trace:
self.tracer.record_calculation_end(variable.name, period, cached_array, **parameters)
Expand All @@ -153,11 +151,11 @@ def calculate(self, variable_name, period, **parameters):
self.max_nb_cycles = max_nb_cycles

# First, try to run a formula
array = self._run_formula(variable, entity, period, extra_params, max_nb_cycles)
array = self._run_formula(variable, entity, period, max_nb_cycles)

# If no result, try a base function
if array is None and variable.base_function:
array = variable.base_function(holder, period, *extra_params)
array = variable.base_function(holder, period)

# If no result, use the default value
if array is None:
Expand All @@ -167,7 +165,7 @@ def calculate(self, variable_name, period, **parameters):
if max_nb_cycles is not None:
self.max_nb_cycles = None

holder.put_in_cache(array, period, extra_params)
holder.put_in_cache(array, period)
if self.trace:
self.tracer.record_calculation_end(variable.name, period, array, **parameters)

Expand Down Expand Up @@ -240,7 +238,7 @@ def trace_parameters_at_instant(self, formula_period):
self.tracer
)

def _run_formula(self, variable, entity, period, extra_params, max_nb_cycles):
def _run_formula(self, variable, entity, period, max_nb_cycles):
"""
Find the ``variable`` formula for the given ``period`` if it exists, and apply it to ``entity``.
"""
Expand All @@ -259,12 +257,12 @@ def _run_formula(self, variable, entity, period, extra_params, max_nb_cycles):
if formula.__code__.co_argcount == 2:
array = formula(entity, period)
else:
array = formula(entity, period, parameters_at, *extra_params)
array = formula(entity, period, parameters_at)
except CycleError as error:
self._clean_cycle_detection_data(variable.name)
if max_nb_cycles is None:
if self.trace:
self.tracer.record_calculation_abortion(variable.name, period, extra_params = extra_params)
self.tracer.record_calculation_abortion(variable.name, period)
# Re-raise until reaching the first variable called with max_nb_cycles != None in the stack.
raise error
self.max_nb_cycles = None
Expand Down
7 changes: 2 additions & 5 deletions openfisca_core/tracers.py
Original file line number Diff line number Diff line change
Expand Up @@ -90,8 +90,6 @@ def clone(self):

@staticmethod
def _get_key(variable_name, period, **parameters):
if parameters.get('extra_params'):
return "{}<{}><{}>".format(variable_name, period, '><'.join(map(str, parameters['extra_params'])))
return "{}<{}>".format(variable_name, period)

def record_calculation_start(self, variable_name, period, **parameters):
Expand Down Expand Up @@ -207,18 +205,17 @@ def print_line(depth, node, value):

return print_line(depth, key, self._get_aggregate(key))

def print_trace(self, variable_name, period, extra_params = None, max_depth = 1, aggregate = False, ignore_zero = False):
def print_trace(self, variable_name, period, max_depth = 1, aggregate = False, ignore_zero = False):
"""
Print value, the dependencies, and the dependencies values of the variable for the given period (and possibly the given set of extra parameters).

:param str variable_name: Name of the variable to investigate
:param Period period: Period to investigate
:param list extra_params: Set of extra parameters
:param int max_depth: Maximum level of recursion
:param bool aggregate: See :any:`print_computation_log`
:param bool ignore_zero: If ``True``, don't print dependencies if their value is 0
"""
key = self._get_key(variable_name, period, extra_params = extra_params)
key = self._get_key(variable_name, period)

def _print_details(key, depth):
if depth > 0 and ignore_zero and np.all(self.trace[key]['value'] == 0):
Expand Down
2 changes: 1 addition & 1 deletion setup.py
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@

setup(
name = 'OpenFisca-Core',
version = '27.0.2',
version = '28.0.0',
author = 'OpenFisca Team',
author_email = 'contact@openfisca.org',
classifiers = [
Expand Down
Loading