-
Notifications
You must be signed in to change notification settings - Fork 11
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
Time issue with demand_eq #44
Comments
wouldn't it be easier if we just did
|
But generally I agree the clarification in the readme is needed |
We can do that, but you need to ensure that the user puts their equation in with regard to the timestep. Also, not all growth rates will be in regard to the operational time step of the simulation. For example you might consider your growth rate annually, but the time step of the simulation might be weeks, or months, or even seconds. In which case the demand equation needs to account for these differences. I suppose this is acceptable, since we're already asking users to understand how to input a equation via eval, so they much have some knowledge of how to write python. I'm just hesitant to put most of the burden on the user, when we could solve the problem on the developer side. |
So from my understanding, we can move forward in two ways:
Option 1 is easier to implement but we lose generality. I personally would be in favor of option 2. Perhaps, we could implement only month-long and year-long time step options (and not provide minute/second options). Thoughts? @jbae11 @FlanFlanagan |
I think the best way is option 2 with default month. If @FlanFlanagan agrees, I'll make a PR about it once #40 gets merged. |
Let's do Option 3: |
Option 3 is my vote. |
okay, I'll make the PR once #47 is merged. |
closed with #46 |
Currently, the
eval function
evaluates the demand_eq state variable inno_inst.py
with the notion that time is in seconds. However, it is unclear to the user what time step length the t variable should be.This issue can be closed when:
dt variable
defined in thecontrol
section of the cyclus input file.The text was updated successfully, but these errors were encountered: