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

Add promoting constructors for Dual number support #199

Merged
merged 1 commit into from
Feb 14, 2024

Conversation

dennisYatunin
Copy link
Member

@dennisYatunin dennisYatunin commented Feb 14, 2024

This PR adds a method to every user-facing ThermodynamicState constructor, in which all input arguments are promoted to the same number type. In addition, q_vap_saturation_generic is given a new method that promotes its input arguments.

With only three exceptions, every function from Thermodynamics.jl that gets called while computing the precomputed quantities and implicit tendencies of ClimaAtmos.jl only takes two input arguments: a parameter set and a ThermodynamicState. The exceptions are

  • constructors of ThermodynamicStates, where some inputs can be prescribed by the conditions at t = 0 or z = 0 while others depend on the prognostic state
  • total_specific_enthalpy(param_set, ts, e_tot), where ts and e_tot both depend on the prognostic state
  • q_vap_saturation_generic(param_set, T, ρ, phase), where T is prescribed by the conditions at z = 0 while ρ depends on the prognostic state in the bottom element

So, this PR constitutes the minimum set of additions to Thermodynamics.jl that will enable us to use automatic differentiation in ClimaAtmos.jl, which involves replacing values in the prognostic state with numbers of type ForwardDiff.Dual.

This is a simpler alternative to #197, which I'm going to close for now. In the future, we may want to come back to the changes in that PR, since they are much more generalizable. That is, they will not require to continue adding new methods to every function that relies on promotion during automatic differentiation in other repositories. The downside to those changes is that they would allow developers to silently introduce Float32/Float64 type instabilities within internal functions, and we would probably want to add more unit tests to ensure that those are avoided.

Copy link

codecov bot commented Feb 14, 2024

Codecov Report

Attention: 26 lines in your changes are missing coverage. Please review.

Comparison is base (1dc23bb) 93.04% compared to head (eccb2ae) 90.98%.

Files Patch % Lines
src/states.jl 0.00% 25 Missing ⚠️
src/relations.jl 0.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #199      +/-   ##
==========================================
- Coverage   93.04%   90.98%   -2.06%     
==========================================
  Files          10       10              
  Lines        1150     1176      +26     
==========================================
  Hits         1070     1070              
- Misses         80      106      +26     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@charleskawczynski charleskawczynski added the Launch Buildkite Add label to launch Buildkite label Feb 14, 2024
@charleskawczynski charleskawczynski added this pull request to the merge queue Feb 14, 2024
Merged via the queue into main with commit c54e008 Feb 14, 2024
18 of 20 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Launch Buildkite Add label to launch Buildkite
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants