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

Injectable Memoization #138

Merged
merged 2 commits into from
Feb 5, 2015
Merged

Injectable Memoization #138

merged 2 commits into from
Feb 5, 2015

Conversation

jiffyclub
Copy link
Member

This enables something like this:

@sim.injectable(autocall=False, memoize=True)
def my_utility_func(x, y, z):
    return x + y + z

@sim.column()
def my_col(another_col, my_utility_func):
    result = my_utility_func(1, 2, 3)
    return another_col * result

And if you call my_utility_func again with the input of 1, 2, 3 you'll get back the cached result instead of re-doing the calculation.

Memoization requires autocall=False and that function inputs always be hashable. The memoization cache follows the same rules as other caching, so you can use cache_scope=, sim.clear_cache, sim.disable_cache, and sim.enable_cache as usual.

If you have a utility function registered as an injectable but
it is not being automatically called you can now get caching of
results based on the inputs so long as the inputs are all hashable.
Use the memoize=True keyword along with autocall=False to get
this behavior.

Note that this only applies if you're using the utility via
the simulation framework (via injection or get_injectable).
@fscottfoti
Copy link
Contributor

Want to merge to master and I'll try it out? Or should I try out and then merge?

@jiffyclub
Copy link
Member Author

I'll merge it, I think it's okay. One thing I noticed is that you're generally not using these utility functions via the simulation framework, you're calling them directly since they are in the global scope. You won't get the memoization going that route, you need to have the function injected by the simulation framework so that you get the version that's been wrapped up with caching.

jiffyclub added a commit that referenced this pull request Feb 5, 2015
@jiffyclub jiffyclub merged commit bb31900 into master Feb 5, 2015
@jiffyclub jiffyclub deleted the memoize-injectable branch February 5, 2015 22:44
@fscottfoti
Copy link
Contributor

That makes sense - I'll take a look.

@fscottfoti
Copy link
Contributor

@jiffyclub
Copy link
Member Author

See line 147 where you're calling parcel_average_price; you're not having parcel_average_price injected, instead you're using it directly from the global namespace. You have decorated parcel_average_price, but the decorator doesn't change the function in the global namespace, it only wraps it within the simulation framework. (This is because when autocall=True the function is wrapped up in a special class and it doesn't make sense to leave that in the global namespace. With autocall=False and memoization it makes more sense to have the wrapped function in the global namespace, but the precedent is set...)

To get the wrapped function from the simulation framework change line 146 to:

@sim.injectable('parcel_sales_price_sqft_func', autocall=False)
def parcel_sales_price_sqft(use, parcel_average_price):
    s = parcel_average_price(use)
    ...

Then the wrapped version of the parcel_average_price function is injected. Similar wherever you use parcel_is_allowed.

@fscottfoti
Copy link
Contributor

OK, that's right. The problem is that use isn't an injectable - it's a parameter that gets called in a for loop - e.g. for use in uses: parcel_average_price_sqft(use) - I'm not sure how I could turn a parameterized function into a for loop. I mean, parcel_average_price is an injectable because there's only one parcel_average_price function, but there are many uses that can get passed so I'm not sure how to do that in the simulation framework. Should I call add_injectable inside the for loop in order to change the use injectable?

@jiffyclub
Copy link
Member Author

I see. Hmm.

One option is to use the things described in the "Running Sim Components a la Carte" section of the docs:

for use in uses:
    sales_price = sim.eval_variable('parcel_sales_price_sqft_func', use=use)

or

for use in uses:
    with sim.injectables(use=use):
        sales_price = sim.eval_variable('parcel_sales_price_sqft_func')

But another thought is whether these functions need to be registered as injectables at all. I might be able to rewire the memoize decorator a little so it can be used on its own, then you'd drop the sim.injectable bit and instead do sim.memoize(cache_scope='iteration') and you'd use the function from the global scope as you've been doing. The downside there is that the function would not be available via injection or get_injectable.

@fscottfoti
Copy link
Contributor

Hmm, I don't love the idea of calling the function from the global scope. I like that the sim framework knows about all the code that gets run for the simulation in a fairly direct way. I think one of the eval_variable solutions should work. So if I do

for use in uses:
sales_price = sim.eval_variable('parcel_sales_price_sqft_func', use=use)

If the variable parcel_sales_price_sqft_func also takes parcel_average_price as an injectable, that will get injected right? So when I set use=use I'm adding additional parameters, but not eliminating the use of injectables that are already registered?

@jiffyclub
Copy link
Member Author

Right, you're temporarily supplementing any already registered stuff.

@fscottfoti
Copy link
Contributor

OK I'll try that!

@jiffyclub
Copy link
Member Author

I've been mulling the idea of iterable injectables. Like you'd register a list and it would serve the contents up one at a time. You could imagine that being used with the 'year' during simulation runs. And if you had two iterable injectables you'd end up with the product of those. But I haven't totally puzzled out all the implications and logistics of that idea.

@fscottfoti
Copy link
Contributor

The thought had occurred to me, especially since I mainly use this for creating columns that are parameterized (e.g. a price for each land use) and could be returned as a dataframe.

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

Successfully merging this pull request may close these issues.

2 participants