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

Optimize out unneeded calls to dss! and lim! #107

Closed
dennisYatunin opened this issue Dec 15, 2022 · 2 comments
Closed

Optimize out unneeded calls to dss! and lim! #107

dennisYatunin opened this issue Dec 15, 2022 · 2 comments

Comments

@dennisYatunin
Copy link
Member

dennisYatunin commented Dec 15, 2022

There are two common situations in which we are doing unnecessary calls to dss! and lim!:

  • If the tableaus a_exp and a_imp start with a row that is all 0s, the value of the first stage is just u. Since u was already DSS-ed at the end of the previous timestep, it does not need to be DSS-ed again. More generally, if the tableaus are such that, on stage i, a_exp[i, 1:i-1] and a_imp[i, 1:i-1] are all 0s, then either there is no implicit solve and the value of stage i is u, or the input to the implicit solve on stage i is u; in both cases, we do not need to DSS u. Note that this reasoning is not valid if users are allowed add callbacks that modify u in ways that require it to be DSS-ed again. So, if we want to optimize out these unnecessary calls to dss!, we should clearly document that users should not add such callbacks. In addition, this reasoning is not valid if users can make the effect of dss! depend on the time at which it was called, which means that we should not pass t as an argument to dss!.
  • If the weights b_exp and b_imp are identical to the last rows of the tableaus a_exp and a_imp (i.e., if the algorithm is FSAL), the value of the last stage is also the final value of u. So, instead of incrementing u with previously-evaluated tendencies and applying lim! and dss!, we can just set u .= U[s]. Once again, this reasoning is not valid if users can make the effects of lim! and dss! depend on the time at which they were called, so we should not pass t as an argument to lim! and dss!. Also, if users can filter U[s] through stage_callback! before evaluating the last explicit tendency, we will need to copy U[s] before it gets filtered so that we can set u .= U[s] afterward (or we could just drop support for filtering through stage_callback! in order to avoid unnecessarily complicated code, in which case users will still be able to filter copies of their state vector "under the hood").
@tapios
Copy link

tapios commented Dec 16, 2022

Let's focus on the code we actually need right now and not write code that is more general than necessary.

  1. All methods we will use have a first row of 0's for the explicit weights a_exp.
  2. There is no case I can think of where we would want to make dss or limiters explicitly time dependent.

@charleskawczynski
Copy link
Member

Closed by #228 and #223

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

No branches or pull requests

3 participants