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

Several Thermodynamics updates. #134

Open
7 of 10 tasks
tapios opened this issue Sep 21, 2022 · 3 comments
Open
7 of 10 tasks

Several Thermodynamics updates. #134

tapios opened this issue Sep 21, 2022 · 3 comments
Assignees
Labels
bugfix 🐛 enhancement New feature or request

Comments

@tapios
Copy link
Contributor

tapios commented Sep 21, 2022

A few updates to Thermodynamics and the documentation:

  • Update the thermodynamics documentation following the tracked changes in the design doc.
  • Complete and merge add Clausius_Clapeyron_relation #112 (including the updated derivative ∂q_vap_sat_∂T)
  • Use the updated function for ∂q_vap_sat_∂T from add Clausius_Clapeyron_relation #112 in saturation adjustment l. 1402. (This will change the gradient by about 5%; thus, it may lead to a minor improvement in convergence of saturation adjustment, but the changes should not be large.)
  • Add a docstring after l. 843 for the saturation_vapor_pressure function over a mix of liquid and ice in l. 914
  • Change the saturation adjustment convergence check generally (in all version of SA) to a solution (i.e., temperature) tolerance, requiring temperature changes ΔT from iteration to iteration to be small in relative terms, i.e., ΔT/T < 1e-4.
  • Enable passing an initial state for saturation adjustment
  • Check average number of iterations needed with previous time step (or stage) as initial state and ΔT/T < 1e-4. Compare that with the current default initial state. (Expectation is that fewer iterations are needed with previous time step as initial state.)
  • If previous test yields expected results (faster convergence with previous timestep as initial state), make that the default in ClimaAtmos.

Some other minor fixes:

  • Use liquid_frac and λ consistently. E.g., replace liquid_frac in ll. 914 ff. and ll. 1314 ff. by λ
  • Replace specific entropy in l. 2846 and l. 2865 by partial pressure
@tapios tapios added enhancement New feature or request bugfix 🐛 labels Sep 21, 2022
bors bot added a commit that referenced this issue Oct 5, 2022
139: Use SolutionTolerance instead of ResidualTolerance r=charleskawczynski a=charleskawczynski

This PR replaces the `RootSolvers.ResidualTolerance` with `RootSolvers.SolutionTolerance`. Closes #135, #134, #130.

Co-authored-by: Charles Kawczynski <[email protected]>
bors bot added a commit that referenced this issue Oct 5, 2022
139: Use SolutionTolerance instead of ResidualTolerance r=charleskawczynski a=charleskawczynski

This PR replaces the `RootSolvers.ResidualTolerance` with `RootSolvers.SolutionTolerance`. Closes #135, #134, #130.

Co-authored-by: Charles Kawczynski <[email protected]>
@charleskawczynski
Copy link
Member

@tapios, I think we spoke about this, the current implementation uses ΔT < tol, not ΔT/T < 1e-4, is this okay? If not, we can add this capability into RootSolvers.jl and update Thermodynamics-- it's not a difficult change, but I just wanted to clarify this point.

Also, can you confirm if we've updated the particular part of the docs in the first bullet point? I believe we've addressed them all in #112

@tapios
Copy link
Contributor Author

tapios commented Oct 10, 2022

@tapios, I think we spoke about this, the current implementation uses ΔT < tol, not ΔT/T < 1e-4, is this okay? If not, we can add this capability into RootSolvers.jl and update Thermodynamics-- it's not a difficult change, but I just wanted to clarify this point.

ΔT < tol is ok if relative tolerance takes much work. But I'd prefer a relative tolerance (e.g., so that we have a relative tolerance on buoyancy, and can easily ran this in other planetary configurations.

Also, can you confirm if we've updated the particular part of the docs in the first bullet point? I believe we've addressed them all in #112

The docs (https://clima.github.io/Thermodynamics.jl/dev/Formulation/) do not seem to be updated yet. There were a number of changes in the LaTeX design doc (tracked) that need to be mirrored in the docs, e.g., formulation of saturation vapor pressure and derivatives of saturation specific humidity. (Downside of having two versions. We should abandon the LaTeX soon.)

@charleskawczynski
Copy link
Member

Ah, we've already added RelativeSolutionTolerance to RootSolvers, so we just need to update it here.

bors bot added a commit that referenced this issue Oct 26, 2022
140: Use RelativeSolutionTolerance r=charleskawczynski a=charleskawczynski

This PR addresses one of the check boxes in #134, by changing from `SolutionTolerance` to `RelativeSolutionTolerance`.

One downside of this, unfortunately, is that it's a breaking change, but I think it's fine if we know that this is what we want in the long run.

Alternatively, we could require users to pass in `::RootSolvers.AbstractTolerance`, which would make the interface less likely to be breaking, however this means that our interface would be tied to using `RootSolvers`, so it's not a complete win to do that.

It looks like all the tests pass with these changes.

Co-authored-by: Charles Kawczynski <[email protected]>
bors bot added a commit that referenced this issue Oct 26, 2022
140: Use RelativeSolutionTolerance r=charleskawczynski a=charleskawczynski

This PR addresses one of the check boxes in #134, by changing from `SolutionTolerance` to `RelativeSolutionTolerance`.

One downside of this, unfortunately, is that it's a breaking change, but I think it's fine if we know that this is what we want in the long run.

Alternatively, we could require users to pass in `::RootSolvers.AbstractTolerance`, which would make the interface less likely to be breaking, however this means that our interface would be tied to using `RootSolvers`, so it's not a complete win to do that.

It looks like all the tests pass with these changes.

Co-authored-by: Charles Kawczynski <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bugfix 🐛 enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants