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

Redesign access to variables and their values in Process subclasses #19

Closed
benbovy opened this issue Nov 10, 2017 · 7 comments
Closed
Milestone

Comments

@benbovy
Copy link
Member

benbovy commented Nov 10, 2017

Having three different properties (i.e., state, change, rate and value as an alias of state) is probably a bad idea:

  • Allowing only one value per variable a-priori makes more sense, especially for a very generic framework.
  • Most of the time only the property value (or state) is used. In a few cases other properties are used as well, e.g., setting property change in .run_step() and updating state in .finalize_step(), but this is less common.
  • I haven't encountered yet any case where more than one of these properties is needed externally (i.e., in other processes). So if we need to deal with different values of the same field (delta, rate, state) inside a process we might simply use regular attributes.
  • Using these properties results in quite verbose code in Process subclasses.
  • Inputs and snapshots are only for the property value (or state).

I therefore suggest that a variable only accepts one value. There is different options for implementing this:

  1. We keep the current design, remove all properties mentioned above and instead provide one property, with a short name like .v.

  2. Maybe it would be possible to get rid of using a specific property to access to the variable value in methods of a Process subclass and instead use directly the attribute to access the value, e.g.,

class Foo(xsimlab.Process):
    coef = xsimlab.Variable(())
    field = xsimlab.Variable((), provided=True)

    def run_step(self, dt):
        self.field = self.coef * dt

Option 2 looks cleaner, actually very much like attrs. It would require a bit of redesign work, though.

Given that we create a model by providing in a dict one or several Process subclasses

model = xsimlab.Model({'foo': Foo})

it should be possible to inspect models directly by using the class attributes, e.g.,

model.foo.coef

The trick to replace the Variable object by their value in Process methods (initialize, run_step, etc.) would be to create instances of each Process subclasses of the model every time we start a simulation. As processes in a model are ordered in a computationally consistent manner, we can just initialize the subclasses in that order so that we can properly link foreign variables.

Overall, option 2 may be actually much cleaner (and possibly more efficient) than the current design.

@benbovy benbovy changed the title Redesign the access to variables and their values in Process subclasses Redesign access to variables and their values in Process subclasses Nov 10, 2017
@benbovy
Copy link
Member Author

benbovy commented Nov 12, 2017

After thinking about option 2, I'm wondering if it shouldn't be better to build this directly on top of attrs instead of re-implementing much of its features. It has both advantages and drawbacks, though. The main risk is adding another required dependency. One of the main advantages is that if it continues to grow in popularity, users may be already familiar with this API.

A xsimlab process would then look very similar to a attrs class:

@xsimlab.process(time_dependent=False)
class Foo(object):
    coef = xsimlab.variable(()) 
    field = xsimlab.variable((), provided=True)
    bar = xsimlab.foreign(Bar, 'bar')

What would change:

  • The class doesn't inherit anymore from a Process base class, instead we use a decorator
  • Some important parameters defining the behavior of a Process (e.g. time_dependent) are given using keyword arguments of the decorator instead of attributes in a class Meta. Other arguments may be available as well, like auto_doc=True as suggested in autogenerate docstrings for Process subclasses (process interface) #3.
  • Variable objects in a process are declared using functions instead of class constructors (e.g., Variable -> variable)
  • All derived classes like FloatVariable would be removed in favour of parametrized validators (see attrs doc). xarray-simlab would provide a bunch of useful validators in addition to those already provided in attrs.
  • optional argument in Variable would be replaced by the specific optional validator in available in attrs.

Like in attrs, we could imagine very succinct names like @xsimlab.p and xsimlab.v, but I'm not particularly a fan of that, especially in this cases we would need short names for foreign variables and variable groups as well, which may be too much short names.

@benbovy
Copy link
Member Author

benbovy commented Nov 17, 2017

Option 2 (i.e., Process instances attributes directly pointing to the values instead of using properties) may work in some cases, e.g., when using numpy arrays and updating these in place, but not in all cases.

So properties might still be needed. But instead of defining a property in Variable (i.e., Variable.value), properties may be defined programmatically when building process classes (i.e., with @xsimlab.process decorator), which would give something equivalent to, e.g.,

class Foo(object):
    @property
    def coef(self):
        return self._xsimlab_coef

    @coef.setter
    def coef(self, value)
        self._xsimlab_coef = value

Another option (without properties) would be to have some logic which auto-update all external references to a variable each time the latter is updated.

@benbovy
Copy link
Member Author

benbovy commented Nov 21, 2017

Using properties, it might be possible to run in the setters all validator or converter functions attached to their respective variables, e.g.,

class Foo(object):
    @property
    def coef(self):
        return self._xsimlab_coef

    @coef.setter
    def coef(self, value)
        # --- run all `Foo.coef`'s validators here ---
        self._xsimlab_coef = value

Note on validators: as value can be either a scalar or an array, it might be worth to provide some default validators that support both (+ broadcasting?).

If we think we don't need to run the validators each time we set a new value (this can be rather done once when creating the inputs, e.g., using the xarray interface), then properties may not be very useful for variables owned by a process and we can use attributes instead. However, properties may still be needed for foreign variables!

@benbovy
Copy link
Member Author

benbovy commented Nov 21, 2017

Using properties would also allow some defensive programming. Property setters are indeed not needed for all variables. Read-only properties would prevent setting the value of variables where this is not allowed (i.e., if provided=False and if variable is not a model input).

The main problem is that whether a variable is an input or not is determined at creation of a Model instance, not at creation of a process class.

At least it is possible to create read-only properties for foreign variables for which provided=False (foreign variables are never model inputs).

EDIT: in some cases we need to be able to update the value of variables that are not inputs nor provided (e.g., main process updating a state variable for which an initial value is provided by another, time-independent process).

@benbovy
Copy link
Member Author

benbovy commented Nov 21, 2017

A possible solution to get the value of a foreign variable from an instance of the process class where the original variable is defined:

class Foo(object):
    def __init___(self):
        self._xsimlab_bar_obj = None

    @property
    def bar(self):
        return self._xsimlab_bar_obj.bar

where self._xsimlab_bar_obj will correspond to the instance of Bar used for one simulation. We'll need to implement some logic to set similar attributes for all process classes in a model, just after these latter are instantiated at the beginning of each new simulation.

@benbovy
Copy link
Member Author

benbovy commented Nov 22, 2017

A alternative name for foreign variables would be link:

@xsimlab.process(time_dependent=False)
class Foo(object):
    coef = xsimlab.variable(()) 
    field = xsimlab.variable((), provided=True)
    bar = xsimlab.link(Bar, 'bar')

@benbovy
Copy link
Member Author

benbovy commented May 7, 2018

Closed in #33

@benbovy benbovy closed this as completed May 7, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant