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

Process keywords #177

Closed
wants to merge 6 commits into from
Closed
Show file tree
Hide file tree
Changes from 4 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
10 changes: 5 additions & 5 deletions vivarium/composites/toys.py
Original file line number Diff line number Diff line change
Expand Up @@ -263,7 +263,7 @@ def next_update(self, timestep, states):
class Proton(Process):
name = 'proton'
defaults = {
'time_step': 1.0,
'timestep': 1.0,
'radius': 0.0}

def ports_schema(self):
Expand Down Expand Up @@ -314,7 +314,7 @@ def next_update(self, timestep, states):
class Electron(Process):
name = 'electron'
defaults = {
'time_step': 1.0,
'timestep': 1.0,
'spin': electron_spins[0]}

def ports_schema(self):
Expand Down Expand Up @@ -667,10 +667,10 @@ def test_composite_parameters() -> None:
composer_parameters = bb_composer.get_parameters()
composite_parameters = bb_composite.get_parameters()
expected_parameters = {
'a1': {'time_step': 1.0},
'a2': {'time_step': 1.0},
'a1': {'timestep': 1.0},
'a2': {'timestep': 1.0},
'a3_store': {
'a3': {'time_step': 1.0}}}
'a3': {'timestep': 1.0}}}
assert composite_parameters == composer_parameters
assert composite_parameters == expected_parameters

Expand Down
99 changes: 63 additions & 36 deletions vivarium/core/process.py
Original file line number Diff line number Diff line change
Expand Up @@ -82,43 +82,51 @@ def _get_parameters(


class Process(metaclass=abc.ABCMeta):
"""Process parent class.

All :term:`process` classes must inherit from this class. Each
class can provide a ``defaults`` class variable to specify the
process defaults as a dictionary.

Note that subclasses should call the superclass init function
first. This allows the superclass to correctly save the initial
parameters before they are mutated by subclass constructor code.
We need access to the original parameters for serialization to
work properly.

Args:
parameters: Override the class defaults. This dictionary may
also contain the following special keys:

* ``name``: Saved to ``self.name``.
* ``_original_parameters``: Returned by
``__getstate__()`` for serialization.
* ``_no_original_parameters``: If specified with a value
of ``True``, original parameters will not be copied
during initialization, and ``__getstate__()`` will
instead return ``self.parameters``. This puts the
responsibility on the user to not mutate process
parameters.
* ``_schema``: Overrides the schema.
* ``_parallel``: Indicates that the process should be
parallelized. ``self.parallel`` will be set to True.
* ``_condition``: Path to a variable whose value will be
returned by
:py:meth:`vivarium.core.process.Process.update_condition`.
* ``time_step``: Returned by
:py:meth:`vivarium.core.process.Process.calculate_timestep`.
"""

defaults: Dict[str, Any] = {}

def __init__(self, parameters: Optional[dict] = None) -> None:
"""Process parent class.

All :term:`process` classes must inherit from this class. Each
class can provide a ``defaults`` class variable to specify the
process defaults as a dictionary.

Note that subclasses should call the superclass init function
first. This allows the superclass to correctly save the initial
parameters before they are mutated by subclass constructor code.
We need access to the original parameters for serialization to
work properly.
self.initialize_parameters(parameters)

Args:
parameters: Override the class defaults. This dictionary may
also contain the following special keys:

* ``name``: Saved to ``self.name``.
* ``_original_parameters``: Returned by
``__getstate__()`` for serialization.
* ``_no_original_parameters``: If specified with a value
of ``True``, original parameters will not be copied
during initialization, and ``__getstate__()`` will
instead return ``self.parameters``. This puts the
responsibility on the user to not mutate process
parameters.
* ``_schema``: Overrides the schema.
* ``_parallel``: Indicates that the process should be
parallelized. ``self.parallel`` will be set to True.
* ``_condition``: Path to a variable whose value will be
returned by
:py:meth:`vivarium.core.process.Process.update_condition`.
* ``time_step``: Returned by
:py:meth:`vivarium.core.process.Process.calculate_timestep`.
"""
def initialize_parameters(
self,
parameters: Optional[dict] = None,
**kwargs
) -> None:
parameters = parameters or {}
if '_original_parameters' in parameters:
original_parameters = parameters.pop('_original_parameters')
Expand Down Expand Up @@ -155,9 +163,28 @@ class can provide a ``defaults`` class variable to specify the
'_emit': True,
'_updater': 'set'}))

self.parameters.setdefault('time_step', DEFAULT_TIME_STEP)
self._set_timestep()

self.schema: Optional[dict] = None

def initialize(
self,
parameters: Optional[dict] = None,
base_parameters: Optional[dict] = None
) -> None:
# base_parameters = parameters.pop('base_parameters')

for local in ['self', '__class__', 'ipdb']:
if local in parameters:
del parameters[local]
parameters.update(base_parameters)
self.initialize_parameters(parameters)

def _set_timestep(self) -> None:
self.parameters.setdefault('timestep', DEFAULT_TIME_STEP)
if self.parameters.get('time_step'):
self.parameters['timestep'] = self.parameters['time_step']

def __getstate__(self) -> dict:
"""Return parameters

Expand Down Expand Up @@ -275,10 +302,10 @@ def calculate_timestep(self, states: Optional[State]) -> Union[float, int]:
"""Return the next process time step

A process subclass may override this method to implement
adaptive timesteps. By default it returns self.parameters['time_step'].
adaptive timesteps. By default it returns self.parameters['timestep'].
"""
_ = states
return self.parameters['time_step']
return self.parameters['timestep']
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

timestep change is good, maybe a separate PR?


def default_state(self) -> State:
"""Get the default values of the variables in each port.
Expand Down
2 changes: 1 addition & 1 deletion vivarium/core/serialize_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,7 @@ def test_serialization_full() -> None:
serialized.pop('function'))
expected_serialized = {
'process': (
"!ProcessSerializer[{'time_step': 1.0, '_name': "
"!ProcessSerializer[{'timestep': 1.0, '_name': "
"'SerializeProcess'}]"
),
'1': True,
Expand Down
38 changes: 26 additions & 12 deletions vivarium/processes/growth_rate.py
Original file line number Diff line number Diff line change
Expand Up @@ -22,11 +22,22 @@ class GrowthRate(Process):
""" A Vivarium process that models exponential growth of biomass """

name = NAME
defaults = {
'default_growth_rate': 0.0005,
'default_growth_noise': 0.0,
'variables': ['mass']
}
# defaults = {
# 'default_growth_rate': 0.0005,
# 'default_growth_noise': 0.0,
# 'variables': ['mass']
# }

def __init__(
self,
default_growth_rate=0.0005,
default_growth_noise=0.0,
variables=('mass',),
**base_parameters):

parameters = locals().copy()
base_parameters = parameters.pop('base_parameters')
self.initialize(parameters, base_parameters)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I know I helped create this thing, but on reflection I feel the cost here for Process creators to have to add this four-line litany to every __init__ method is too high. I appreciate what we're trying to solve here but the trade-off can't be to create cryptic things we demand of our users.... IMO

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe we can make this optional? If people want function keywords, they can follow this template. But the previous __init__ approach with defaults and a parameters dict can work just as well.


def ports_schema(self):
return {
Expand Down Expand Up @@ -73,13 +84,16 @@ def next_update(self, timestep, states):
def test_growth_rate(total_time=1350):
initial_mass = 100
growth_rate = 0.0005
config = {
'variables': ['mass'],
'default_growth_rate': growth_rate,
'time_step': 2,
}

growth_rate_process = GrowthRate(config)
# config = {
# 'variables': ['mass'],
# 'default_growth_rate': growth_rate,
# 'timestep': 2,
# }

growth_rate_process = GrowthRate(
default_growth_rate=growth_rate,
timestep=5.0,
variables=('mass',))
initial_state = {'variables': {'mass': initial_mass}}
experiment = process_in_experiment(
growth_rate_process,
Expand Down