Set thermo state iteration params

[charleskawczynski]

This PR changes our thermodynamic state iteration parameters to use

• a maximum of 1 iteration
• a tolerance of 3 K

[bischtob]

are we ready? :)

[tapios]

LGTM if it passes tests. (And if it passes, can we make the tolerance even smaller, e.g., 2 K?)

[charleskawczynski]

LGTM if it passes tests. (And if it passes, can we make the tolerance even smaller, e.g., 2 K?)

It's not passing / working with some experiments. The following break:

• held suarez (ρe_tot) equilmoist topography
• baroclinic wave (ρe_tot) equilmoist topography
• Compresible single column EDMF DYCOMS_RF01

There are some behavioral changes in other examples, but those are non-blocking

[tapios]

LGTM if it passes tests. (And if it passes, can we make the tolerance even smaller, e.g., 2 K?)

It's not passing / working with some experiments. The following break:

• held suarez (ρe_tot) equilmoist topography
• baroclinic wave (ρe_tot) equilmoist topography
• Compresible single column EDMF DYCOMS_RF01

There are some behavioral changes in other examples, but those are non-blocking

What does "break" mean here? (There may be small changes in those, of course. We do not have exact solutions for them.)

[charleskawczynski]

What does "break" mean here? (There may be small changes in those, of course. We do not have exact solutions for them.)

We error in saturation adjustment, so the solution at the end of the iteration is larger than 3 K. Here's one example:

  | maxiter reached in saturation_adjustment:
  | Method=NewtonsMethod, e_int=28262.293520894877, ρ=1.134188857091223, q_tot=0.009, T=283.80521815319395, maxiter=1, tol=2152.518070462876
  | ERROR: LoadError: TaskFailedException
...

Our options seem to be:

• Try further increasing the temperature tolerance
• Try increasing maxiter
• Thread these parameters throughout the model and expose them to the command line args and make them job dependent (we'd probably want to make these non-optional arguments so that we don't accidentally have inconsistent parameters)

That said, I suspect fixing #795 will give us a bigger performance gain than reducing the number of iterations alone. Are we sure that now is the best time to make this performance/accuracy trade-off? My recommendation would be to make these changes after things are a bit more stable--especially if twiddling this parameter impacts stability (which, IIRC, it did back when experimenting with this in TC, cc @yairchn)

[tapios]

• Try further increasing the temperature tolerance
• Try increasing maxiter
• Thread these parameters throughout the model and expose them to the command line args and make them job dependent (we'd probably want to make these non-optional arguments so that we don't accidentally have inconsistent parameters)

Try to see if maxiter = 2 resolves this. If not, increase temperature tolerance.

That said, I suspect fixing #795 will give us a bigger performance gain than reducing the number of iterations alone. Are we sure that now is the best time to make this performance/accuracy trade-off?

The best time to make this change was a year ago; second best time is now. At high resolution, all will be waiting for the point with the slowest convergence...

[tapios]

A slightly bigger issue, which may affect accuracy, is that there's a term missing in the derivative of the saturation specific humidity. I'll add it sometime soon.

[tapios]

And one more point: The way this is usually done (and what we really should be doing) is to use the temperature at the previous time step as the initial condition for the iterations. I mentioned this a few times, and it got into the purity of code design. But is there any serious reason why we cannot pass in an initial guess for the iterations as an input argument, and use the previous time step temperature as that initial guess.

I guarantee with that we'll be fine with 1 iteration.

[charleskawczynski]

And one more point: The way this is usually done (and what we really should be doing) is to use the temperature at the previous time step as the initial condition for the iterations. I mentioned this a few times, and it got into the purity of code design. But is there any serious reason why we cannot pass in an initial guess for the iterations as an input argument, and use the previous time step temperature as that initial guess.

I guarantee with that we'll be fine with 1 iteration.

We have an issue for it, I think it's just not been prioritized

[tapios]

Would be good to do. Given how much time we've spent discussing convergence issues, it'll probably pay quickly to get it done.

[trontrytel]

This has gone a little stale. Do we still want those changes? @szy21 @charleskawczynski

[szy21]

I think we want it but it just didn't work :(

[tapios]

We want maxiter = 1 in regular GCM runs. Are we still not doing this? If so, we should get to the bottom of it and finish it (with the previous stage/time step as initial guess). Again, this works in all other GCMs I know and should work for us.

[szy21]

We want maxiter = 1 in regular GCM runs. Are we still not doing this? If so, we should get to the bottom of it and finish it (with the previous stage/time step as initial guess). Again, this works in all other GCMs I know and should work for us.

No, we are not doing this. I can look into it when I have some more time, unless someone else wants to.

[trontrytel]

I can take this on. Since I have unearthed the Thermodynamics PRs

[szy21]

I can take this on. Since I have unearthed the Thermodynamics PRs

haha, thanks!

[charleskawczynski]

Superseded by #1648