New approach to normalising variables in SNES#3395
Conversation
Has involved refactoring Solver to allow loading/saving from/to arbitrary sets of fields. Have also changed how scale_vars works, although I'll probably want to name a different setting for this. What I've made it done doesn't appear to be what it was intended for originally.
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| // Copy derivatives back | ||
| { | ||
| if (diagnose) { | ||
| BoutReal* fdata = nullptr; |
There was a problem hiding this comment.
warning: pointee of variable 'fdata' of type 'BoutReal *' (aka 'double *') can be declared 'const' [misc-const-correctness]
| BoutReal* fdata = nullptr; | |
| { const |
There was a problem hiding this comment.
No it can't, because the argument of save_derivs must be non-const.
| } | ||
|
|
||
| if (diagnose) { | ||
| BoutReal* resid_data = nullptr; |
There was a problem hiding this comment.
warning: pointee of variable 'resid_data' of type 'BoutReal *' (aka 'double *') can be declared 'const' [misc-const-correctness]
| BoutReal* resid_data = nullptr; | |
| { const |
There was a problem hiding this comment.
No it can't, because the argument of save_derivs must be non-const.
| class SNESSolver; | ||
|
|
||
| #include <vector> | ||
|
|
There was a problem hiding this comment.
warning: included header vector is not used directly [misc-include-cleaner]
There was a problem hiding this comment.
Yes it is, see lines 281 and 283.
| } | ||
|
|
||
| void Solver::set_id(BoutReal* udata) { loop_vars(udata, SOLVER_VAR_OP::SET_ID); } | ||
| void Solver::set_id(BoutReal* udata) { |
There was a problem hiding this comment.
warning: method 'set_id' can be made static [readability-convert-member-functions-to-static]
include/bout/solver.hxx:602:
- void set_id(BoutReal* udata);
+ static void set_id(BoutReal* udata);There was a problem hiding this comment.
No it can't. This calls loop_vars which isn't static (and can't be made static).
| } | ||
|
|
||
| void Solver::set_id(BoutReal* udata) { loop_vars(udata, SOLVER_VAR_OP::SET_ID); } | ||
| void Solver::set_id(BoutReal* udata) { |
There was a problem hiding this comment.
warning: pointer parameter 'udata' can be pointer to const [readability-non-const-parameter]
include/bout/solver.hxx:602:
- void set_id(BoutReal* udata);
+ void set_id(const BoutReal* udata);| void Solver::set_id(BoutReal* udata) { | |
| void Solver::set_id(const BoutReal* udata) { |
There was a problem hiding this comment.
Again, no it can't. It gets passed into a function that needs it to be non-const.
Many of these suggestions were just wrong. I'm not sure what's going on there. |
|
Nice work, Chris! Thank you for doing this. I will check it more carefully next week. One thing I was wondering about is how this behaves for sign-changing variables. For example, the ion parallel velocity, Vd+, can be positive, negative, or very close to zero. So, the scale based on abs(x) could potentially become very small. Is there a way to treat sign-changing variables differently? |
|
Variables are always normalised by their absolute value, so negative variables will stay negative. We also clamp the normalisation, limiting it to |
C is a template parameter so can be determined at compile time.
The x1 vector is only allocated if predictor is enabled.
Previously metadata_it only advanced on the non-continue path. If one field had evolve_bndry == false, every later field in that boundary loop would be checked against the same stale metadata entry and skipped too.
When SNES takes zero iterations (nl_its == 0) a simple Euler step is taken. If scale_vars is true then the derivatives should be scaled to be consistent with rhs_function.
3 participants
The
rescale_varsoption inSNESSolverhas been rewritten. Previously it was meant to perform Jacobian column scaling. It now changes the nonlinear equation being solved fromto
where$f$ is the function for the time-derivative of $x$ , $\tilde x = Sx$ , and $S$ is a diagonal matrix with each entry set to the inverse of the corresponding value of $x$ in a previous time-step.
A new, dynamic scheme has been adopted for deciding when to rescale$x$ , to ensure rescaling will happen promptly when $x$ has changed significantly.
With this PR, the SNES solver will also output the residual from the last nonlinear solve (i.e., the value of$F(x)$ , not the value of $f(x)$ ) when
diagnose = true. This was used to help understand what areas of the domain were controlling the convergence of the solve and whether rescaling changed this. In order to output this information, some refactoring was done to the baseSolverclass to generalise its functions for transferring data between arrays andFieldobjects.