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

Dynamic stopping by setting FINAL_TIME #278

Closed
gingergenius opened this issue Jul 2, 2021 · 8 comments
Closed

Dynamic stopping by setting FINAL_TIME #278

gingergenius opened this issue Jul 2, 2021 · 8 comments

Comments

@gingergenius
Copy link

gingergenius commented Jul 2, 2021

I have a dynamic stopping condition that works in Vensim.

@cache.step
def final_time():

    return if_then_else(
            abs( 
                stock() - threshold()
            ) 
            <= 5,
            lambda: time(),
            lambda: max_final_time()
    )

If a value reaches a certain threshold, the model stops. If it doesn't reach the threshold, the model runs until the maximum time.

I verified that FINAL_TIME is being set as expected and that the difference between TIME and FINAL_TIME becomes zero at some point. The model, however, just goes on running until max_final_time.

@enekomartinmartinez
Copy link
Collaborator

Hi @gingergenius ,
Currently, the final time is set at the beginning of the simulation as a constant value. Its value is used to compute the return_timestamps. As at the beginning of the simulation the condition is false, the final time taken for all the simulation will be max_final_time.

In order to make conditions like yours work some fixes will be needed. Probably we can adapt the Time object in functions.py to keep computing the value of the control variables during the integration and manage it. This would allow to include non-static final_time, time_step or saveper values.

The importance of using a class and not simply modify the integrator is that we need to keep working the models were return_timestamps will set the final time.

I don't have time to solve this problem at the moment and I don't know when I will have time to do it. For the time being, if you can't solve the problem yourself, I recommend that you set an approximate end time that will ensure that the condition is met.

@gingergenius
Copy link
Author

I decided to just crop the dataframe manually like this:

    time_difference = stocks["FINAL TIME"] - stocks["TIME"]
    final_time_is_time = time_difference.loc[time_difference == 0].index
    
    print(f"Time needed: {final_time_is_time[0]} months")
    
    stocks = stocks.loc[:final_time_is_time[0]]

Everything after the first occurence of TIME == FINAL_TIME is disregarded.

@gingergenius
Copy link
Author

@enekomartinmartinez unfortunately version 1.7.0 seems to break my currently functional workaround.

FINAL TIME does not change when the dynamic stopping condition should fire.
Could it be that my equations for FINAL TIME are overwritten by PySD?

@enekomartinmartinez
Copy link
Collaborator

Hi @gingergenius ,
Sorry for that, in order to have consistent information in the output with the time step for the reload functionality I had to rewrite the final time values with the ones introduced by the user when calling run or by the one used to compute return_timestamps. That forced me to overwrite the output values of final time.

The variable final_time is not modified during integration, but at the end when the outputs are exported. Then, if you define

STOP VARIABLE =
    FINAL TIME ~~|

the STOP VARIABLE will save the values of FINAL TIME at each time step. Then, you only need to check STOP VARIABLE values.

I will try to solve the thing that produced the bug, so you can continue using the FINAL TIME. But if needed, I recommend you to do as above.

When I have time, maybe in September, I may work on big changes on PySD, such as how the cache is assigned, in order to improve efficiency and reduce needed resources. I will also check how to manage time, final time and time step, so the model can be automatically stoped with a condition as it is yours.

@enekomartinmartinez
Copy link
Collaborator

Hi again @gingergenius ,
I corrected last mentioned bug in PR #282.
Moreover, I added the automatically stop of the integrention when time >= final_time. Therefore, you will not need to subset the results.

As soon as tests pass I will merge that PR and create a new release to make it available in pip and conda-forge.

@gingergenius
Copy link
Author

You are crazy quick with your corrections, thanks a ton!

@enekomartinmartinez
Copy link
Collaborator

You welcome,
It would be necessary to review well the control variables in PySD in a future. But for now, I think the changes will be enough to make it work well with your model.

@gingergenius
Copy link
Author

You are crazy quick with your corrections, thanks a ton!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants