-
Notifications
You must be signed in to change notification settings - Fork 221
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
(Re-)add support for "freezing" constants in state updaters #480
Comments
I somehow missed this issue when you made it. I think it's a good idea that it should be an option, but I'm not at all sure what the best default should be, freeze or not freeze. Which do we think is the more problematic: long startup times or excessive recompilations for multiple runs with different parameters? The ideal solution might be that freezing is off by default, and if the solution with freezing off takes too long (e.g. more than a few seconds) it raises a warning and tells you to try turning freezing on. |
If the main issue is whether or not constants are positive, perhaps an intermediate solution would be that we check for each unfrozen constant if it is positive or not, and include that when computing the solution. If you had lots of constants that were sometimes positive and sometimes negative, you'd got lots of recompilations, but often a constant will keep its sign even if its magnitude changes. |
I think coming up with specific solutions is hard and will be a lot of work. I used the sign of constants as an example, but it could also be the sign of an expression involving constants, etc. My gut feeling is that sympy's symbolic computations are more time-consuming than re-compilations, but that will change if we cache the state update step. It will still make a difference for the first time you use some equations or for interactive use. One option is that we use the system I suggested for #481 and have method-specific defaults. For example, the simple Euler update would not freeze constants because it does not take any significant time to come up with the statements but the linear state updater would default to freezing since it can make a significant time difference here (or might even allow to integrate equations that couldn't be integrated otherwise). |
What do you mean about specific solutions being hard? For the positive/negative constants thing, I just meant that you can pass that information to Sympy (since at the time we are sending something to Sympy we already know the value). That might lead to a big improvement in symbolic computation time and not introduce too much recompilation. Method-specific defaults is OK but maybe confusing for the user? |
Currently, our state updaters operate fully symbolically, i.e. they keep the constant names in the generated code (but the code generation stage might replace them later on, e.g. in standalone). This has the advantage that we don't need to re-compile code when constants change but it has the disadvantage that state updaters that rely on sympy for symbolic calculations (e.g. the linear state updater) either take longer than necessary or may not be able to integrate equations at all (for example, because it might be important whether a constant is positive or not). We should give users the choice to switch this feature on (or maybe have it on by default)?
The text was updated successfully, but these errors were encountered: