-
Notifications
You must be signed in to change notification settings - Fork 14
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
Make Copies of Calculators Optional #70
Comments
@andersonfrailey, thanks a lot for raising an issue about memory usage in Behavioral-Responses. Right, we are making the copies to ensure this promise: Specifically, the deep copies ensure that the response function doesn't call So your suggestion makes sense in that it would allow users to explicitly choose whether to allow Behavioral-Responses to calculate their calc_1 and calc_2 or not. They get to ask themselves, do I want a permanent small memory effect or a temporary memory explosion? On the other hand, adding this option will definitely add some complexity to the code and the user experience. Is it better to just accept and document that a side effect of behavior is calculating your calculators, and then get rid of those two copies for everyone? (I agree with your assessment that we should keep the third). Interested in other perspectives here. Also happy to review a PR demonstrating that giving the user the option doesn't add much complexity. |
@MattHJensen, thanks for the response. I'll work on a PR that would make the copies optional to see how that affects complexity. For backwards compatibility purposes I'll make it so that the copies are still made by default and the user has to specify they don't want the copies made. |
What do you all think about using a keyword argument, I benchmarked the aggregate memory usage using
using this function: def test_func(inplace):
rec = tc.Records.cps_constructor()
refyear = 2020
assert refyear >= 2018
reform = {'II_em': {refyear: 1500}}
# ... construct pre-reform calculator
pol = tc.Policy()
calc1 = tc.Calculator(records=rec, policy=pol)
calc1.advance_to_year(refyear)
# ... construct two post-reform calculators
pol.implement_reform(reform)
calc2d = tc.Calculator(records=rec, policy=pol) # for default behavior
calc2d.advance_to_year(refyear)
# Keep track of some of the variables that will be modifed
# in response if inplace is True.
df_before_1 = calc1.dataframe(tc.DIST_VARIABLES)
df_before_2 = calc2d.dataframe(tc.DIST_VARIABLES)
behresp.response(calc1, calc2d, elasticities={"inc": -0.2}, dump=True, inplace=inplace)
# Grab the same variables after.
df_after_1 = calc1.dataframe(tc.DIST_VARIABLES)
df_after_2 = calc2d.dataframe(tc.DIST_VARIABLES)
# If inplace is True, the variables should have been modified.
# If inplace is False, response only modified a copy of calc_1
# and calc_2.
if inplace:
assert not df_before_1.equals(df_after_1)
assert not df_before_2.equals(df_after_2)
else:
assert df_before_1.equals(df_after_1)
assert df_before_2.equals(df_after_2) which I called like this: # obtain high-level stats on memory usage
%memit test_func(inplace=False) At the bottom of this comment, you can find a line-by-line breakdown to see how memory changed in each line of You can replicate these results with this notebook. Note that the numbers will vary some with each run but the general memory usage stats should remain similar. With
With
|
@MattHJensen I'm trying to run Tax-Brain with behresp on my 16GB RAM limited laptop and it's reminding me of this issue. I still think it would be helpful to be able to run behresp without making so many copies of |
Currently, Behavioral-Responses makes three copies of the calculator objects passed as arguments to the
response
function. The first two are made right when the function starts running, and the third occurs further along when adding the behavioral-response changes to income.It's clear why we would want to make a copy before changing income, and I can see an argument for why we would want to make copies of the original copies from the beginning. However, could we add an argument to the responses function that makes creating these copies optional? From what I can see, there are no modifications being made to the
calc1
andcalc2
variables, only the copy ofcalc2
. Of course I could be missing something obvious.Making the copies optional would significantly reduce the amount of memory needed for partial equilibrium simulations, which would be especially helpful in Tax-Brain.
cc @hdoupe
The text was updated successfully, but these errors were encountered: