Conversation
Integrate the MUSICA-MICM ODE chemistry solver with the MPAS atmosphere core. MICM initialization, time-stepping, and finalization are called through the existing chemistry hooks in mpas_atm_chemistry.F. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
|
||
| <var name="qB" array_group="passive" units="kg kg^{-1}" | ||
| description="Atomic B mixing ratio"/> | ||
|
|
There was a problem hiding this comment.
These variables are declared here in the Registry.xml file for the init_atmosphere core, but it doesn't look like there's any corresponding code changes to actually provide values for them. Is there some offline script that's used to initialize the qAB, qA, and qB fields? Or are they just expected to be zero in the initial conditions?
There was a problem hiding this comment.
I use an offline python script to initialize the test tracers, e.g. to a sinusoidal pattern.
There was a problem hiding this comment.
For symmetry with core_atmosphere/Registry.xml, these have also been moved to the end of the scalars var_array with array_group="chem_test" in 7a15848.
|
|
||
| <var name="qB" array_group="passive" units="kg kg^{-1}" | ||
| description="Atomic B mixing ratio"/> | ||
|
|
There was a problem hiding this comment.
In principle, the order shouldn't matter too much, but it might be nicer to add these at the end of the scalars var_array. We could also give them a more descriptive group name, perhaps something like chem_test?
There was a problem hiding this comment.
Moved to the end of scalars with array_group="chem_test" in 7a15848.
|
|
||
| <var name="tend_qB" name_in_code="qB" array_group="passive" units="kg m^{-3} s^{-1}" | ||
| description="Tendency of atomic B mass per unit volume divided by d(zeta)/dz"/> | ||
|
|
There was a problem hiding this comment.
Similar to the comment for the scalars var_array, perhaps we can add these at the end of the var_array using the chem_test array group?
There was a problem hiding this comment.
Moved to the end of scalars_tend with array_group="chem_test" in 7a15848.
| !------------------------------------------------------------------------- | ||
| module mpas_atm_chemistry | ||
|
|
||
| use mpas_kind_types, only : StrKIND, RKIND |
There was a problem hiding this comment.
It doesn't look like StrKIND is only used in chemistry_init, where there's already an use statement to import StrKIND.
Also, it looks like RKIND is only used in the chemistry_step routine. Would it be worth moving the use statement to import RKIND to chemistry_step for consistency with the import of real64?
There was a problem hiding this comment.
Good catch. The module-level use mpas_kind_types was redundant (chemistry_init already imports StrKIND locally, and RKIND is only needed in chemistry_step). In 7a15848 the module-level import is removed and use mpas_kind_types, only : RKIND is moved into chemistry_step.
| ! TEMPORARY: ABBA specific initial concentrations | ||
| real(real64), allocatable :: init_ab(:) | ||
| real(real64), allocatable :: init_a(:) | ||
| real(real64), allocatable :: init_b(:) |
There was a problem hiding this comment.
It doesn't look like these variables are actually used. Is there otherwise a reason for declaring them here?
There was a problem hiding this comment.
These were placeholders for initial-concentration seeding that had not been wired up yet. In 7a15848 they are replaced with a single reused initial_profile(:) populated per species in a loop over state%species_ordering, using micm%get_species_property_double() with the INITIAL_CONCENTRATION_PROPERTY constant. MICM species concentrations are now initialized from the __initial concentration field in the MICM YAML, so the hard-coded ABBA scaffolding is gone.
| real(real64), allocatable :: init_a(:) | ||
| real(real64), allocatable :: init_b(:) | ||
|
|
||
| character(len=*), parameter :: INITIAL_CONCENTRATION_PROPERTY = "__initial concentration" |
There was a problem hiding this comment.
It doesn't look like this parameter is used anywhere.
There was a problem hiding this comment.
Now used in 7a15848 as the property key passed to micm%get_species_property_double() to read initial concentrations from the MICM YAML. See the reply on the init_ab/init_a/init_b thread for the full pattern.
|
@dwfncar Is there a MICM configuration file that's needed in order to run with the ABBA mechanism? If so (and assuming it's short), could you include the contents of that configuration file in the PR description? |
- Registry (core_atmosphere, core_init_atmosphere): move qAB/qA/qB and tend_qAB/tend_qA/tend_qB to the end of their scalars/scalars_tend var_arrays with array_group="chem_test" (per mgduda review comments). - mpas_atm_chemistry.F: move RKIND import from module level into chemistry_step, where it is the only consumer. - mpas_musica.F: wire up INITIAL_CONCENTRATION_PROPERTY through micm%get_species_property_double() so initial species concentrations from the MICM YAML (__initial concentration) are loaded into state%concentrations at init. Replace unused init_ab/init_a/init_b placeholders with a single reused initial_profile(:). Also call assign_rate_parameters after the species loop so USER_DEFINED reactions in the ABBA mechanism have a defined rate. Smoke test on the supercell case now shows physically correct AB -> A + B decay with atom conservation. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
musica_step now treats non-convergence and early termination as fatal errors that set error_code and return with a descriptive error_message, instead of just logging a warning. The caller (mpas_atm_chemistry:chemistry_step) already routes non-zero error_code through MPAS_LOG_CRIT, so the model now aborts on a silent MICM failure rather than advancing with stale state. Two checks added: - solver_state must be 'Converged' (previously only log-warned) - solver_stats%final_time() must reach the requested time_step (catches partial/interrupted solves that would otherwise be invisible) log_solver_stats is now called before the checks so the stats leading to a failure are always in the log. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
Done — the full |
|
@mgduda Thanks for the review. All six review threads addressed in 7a15848; individual replies left on each. One extra, unsolicited change in a follow-up commit (752f895) that I want to flag because it changes semantics: |
Added the MUSICA-MICM ODE chemistry solver.
MICM initialization, time-stepping, and finalization are called through
the existing chemistry hooks in mpas_atm_chemistry.F.
ABBA mechanism is hard coded for testing.
ABBA MICM configuration
The ABBA mechanism used for testing is two USER_DEFINED reactions (a
forward dissociation
AB → A + Band a reverse recombinationA + B → AB) with prescribed scaling factors.abba.yaml:Smoke test
Built on Ubuntu with the conda-forge
mpasenv (gfortran/ openmpi / netcdf / pnetcdf) andMUSICA=true, linked against MUSICA-Fortran 0.14.5 / MICM 3.11.0. Ran the supercell idealized case withconfig_micm_file='abba.yaml', 30 s simulated time, 8 MPI ranks. Results:Atom conservation
[X] + [AB] = 1.0holds exactly for X ∈ {A, B}. Solver: 9 steps, 9 accepted / 0 rejected,final_time = 3.0 smatchesconfig_dt. Zero model errors.