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

sim.model as a class? #117

Open
bridwell opened this issue Sep 24, 2014 · 3 comments
Open

sim.model as a class? #117

bridwell opened this issue Sep 24, 2014 · 3 comments

Comments

@bridwell
Copy link
Contributor

One thing I'm noticing w/ the sim.model wrappers is it seems that models are being re-initialized for each year in the simulation. This seems undesirable since initialization is then being called many times over. For example:

@sim.model('household_transition_model')
def household_transition_model(households, year, controls_df, control_col_name):
    t = transition.TabularTotalsTransition(controls_df, control_col_name)
    tm = TransitionModel(t)
    updated, added, updatedLinks = tm.transition(households, year)
    # ...

I've gotten around this by creating an instance of the model outside of the model wrapper and then injecting it, for example:

t = transition.TabularTotalsTransition(controls_df, control_col_name)
tm = TransitionModel(t)
sim.add_injectable('hh_transition', tm)

@sim.model('household_transition_model')
def household_transition_model(hh_transition, households, year):
    updated, added, updatedLinks = hh_transition.transition(households, year)
    # ...

Is there a better way to do this? Declaring the model themselves as injectables certainly works but it feels a little cumbersome. It might be nice if we could decorate a callable class. Then we could define initialization logic in the init method to create model instances and then run year specific code in the callable method?

thanks,
scott

@jiffyclub
Copy link
Member

Adding the model as an injectable is 100% in the spirit of urbansim. An alternative to the way you've set it up is to use the @injectable decorator for a function and cache the result so it's only run once:

@sim.injectable('hh_transition', cache=True)
def hh_transition():
    t = transition.TabularTotalsTransition(controls_df, control_col_name)
    return TransitionModel(t)

You can if you like set up a callable class. Right now you can't decorate a class because the model decorator doesn't know about classes and that they
must be instantiated before they can be used. But you can instantiate a class on your own and add it as a model via sim.add_model.

@bridwell
Copy link
Contributor Author

I did some playing around w/ this, if we update the init in _ModelFuncWrapper from:

def __init__(self, model_name, func):
    self.name = model_name
    self._func = func
    self._arg_list = set(inspect.getargspec(func).args)

To:

def __init__(self, model_name, func):
    self.name = model_name
    if inspect.isfunction(func):
        self._func = func
        self._arg_list = set(inspect.getargspec(func).args)
    else:
        if callable(func):
            # create an instance of the model class w/ the injectables
            init_args = set(inspect.getargspec(func.__init__).args)
            init_args = init_args - set(['self'])
            kwargs = _collect_injectables(init_args)
            self._func = func(**kwargs)

            # define the callabale arguments for year specific actions
            self._arg_list = set(inspect.getargspec(func.__call__).args)
            self._arg_list = self._arg_list - set(['self'])

Then we can can decorate models as classes w/ both initialization and run-year logic. Inputs to both the model initialization and run are all injectables:

@sim.model('household_transition_model')
class HouseholdTransitionModel(object):
    def __init__(self, control_totals, control_col_name):
        t = transition.TabularTotalsTransition(controls_totals, control_col_name)
        self.transition_model = TransitionModel(t)

    def __call__(self, year, households):
        updated, added, updatedLinks = self.transition_model(households, year)

Is this something you can support? I personally feel it makes the models a little easier to set up and organize. But perhaps there are some gotachas with this approach that I'm not recognizing?

@jiffyclub
Copy link
Member

I'm okay with supporting something along those lines, though if we're going to support models as classes we'll need to look into doing the same for injectables and tables.

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

2 participants