Implement a PotentialTemperatureThermodynamics formulation #218
Conversation
glwagner
changed the title
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
test/formulations.jl
Outdated
| @testset "Time-stepping AtmosphereModel with $thermo_name thermodynamics" begin | ||
| formulation = AnelasticFormulation(reference_state, thermodynamics=thermo_name) | ||
| model = AtmosphereModel(grid; thermodynamic_constants=constants, formulation) | ||
| thermo_type = eval(Symbol(thermo_name, :Thermodynamics)) |
There was a problem hiding this comment.
Why not passing the concrete object in the for loop instead of using eval? 🥲
There was a problem hiding this comment.
Fair question... I tentantiavely decided to make the user interface with Symbols. The reason is because, within the user interface, the materialization of the "Thermodynamics" object requires information that is passed to the AtmosphereModel constructor.
The objective is to allow this to be a one line change to switch between formulations.
In the test, I want to stress that user interface. But this is def up for debate.
There was a problem hiding this comment.
Another option is to use stub objects, eg
thermodynamics = PotentialTemperatureThermodynamics()
and then later we call
formulation = materialize_formulation(user_formulation, grid, user_boundary_conditions)
we need a show method because the output is humungus; then we can make a doctest?
Co-authored-by: Mosè Giordano <765740+giordano@users.noreply.github.com>
This reverts commit 235feb4.
…calEarth/Breeze.jl into glw/potential-temperature
|
we are close! |
Co-authored-by: Mosè Giordano <765740+giordano@users.noreply.github.com>
|
Finally all green, only sad thing is that coverage dropped from 74% to 68%, lots of new code is untested: #218 (comment) |
|
I can add tests here\ |
navidcy
left a comment
navidcy
left a comment
There was a problem hiding this comment.
Should we add something in the docs describing this alternative formulation?
|
@giordano looks like we have better code coverage now! We can keep working on this in the future. |
This PR starts implementing an alternative to StaticEnergyThermodynamics that uses the potential temperature density as a prognostic variable. This is a precursor to also supporting EntropyThermodynamics. It depends on #216 and #217