Skip to content

Pull request to put the Isca Mars configurations into Isca's master - #299

Open
sit23 wants to merge 107 commits into
ExeClim:masterfrom
sit23:mars_dev_gfort_local_heating_2026
Open

Pull request to put the Isca Mars configurations into Isca's master#299
sit23 wants to merge 107 commits into
ExeClim:masterfrom
sit23:mars_dev_gfort_local_heating_2026

Conversation

@sit23

@sit23 sit23 commented Aug 4, 2026

Copy link
Copy Markdown
Contributor

The Isca Mars configurations haven't ever been merged into the master, and I'd like to make sure this is done ASAP.

In this P/R I have put in my grey-radiation Mars and Socrates Mars configurations without dust as test cases that can be run with the trip tests. These are the ones that are described in this paper:

https://www.mdpi.com/2073-4433/10/12/803

I'd also like to get in Emily Ball's Isca-Mars configurations that built on my Mars stuff, but I wanted to do this first. This P/R is ready for merging (pending passing the trip tests).

sit23 and others added 30 commits May 11, 2018 12:18
…my module in order that we can correctly calculate rrsun using the true anomaly rather than the mean anomaly. Have also added multiplication by rrsun to two-stream-grey-rad, which makes no difference to results in initial tests when ecc = 0.0
…ue anomaly, and therefore be able to do mars time-telling.
… expects orbital_period in seconds, but I had supplied it in days. This is because Alex's code expects it in days. Will fix Alex code to make it consistent.
…e of surface optical depths and albedos, assuming transparancy in the visible. Useful for working out rough values for optical depths and albedos for a simple mars model.
…ay is not the same as one mars day. Have altered rotation rate and orbital rate such that each can be set as an integer, and we end up with a length of sol which is also an integer. This can then be used as the averaging period.
…dependently. Then also only calculating rh when rh is asked for. Model now runs and looks vaguely mars-like, but dates of equinox etc still not right.
…nml, and ability to use specified temperatures for evaporation calculations in surface flux. This means that, when using a dry model where temperatures are outside standard range for sat specific humidity calculations, then false temps can be used to stop it failing.
Cores on a Mac cannot be tied in the same way that they can on linux, so core affinity is not possible.  Here I am just stubbing out the core affinity functions to mimic the linux case where it cannot get the affinity info.

More info in links inline in `affinity.c`.
…xis. For some reason, the interpolator will get rid of such variables when used on its own, so I modified the python to add them back.
…o idealized moist phys back in teh old GFDLMoistmodel repo on the local_heating_dev branch. This seems to work now, and should hopefully be useful. Only tried the Isidoro option, and not the input file option, but this should be fine.
Adding mac functionality to local heating branch
…cal prescribed heating input files. Seems to work alright, but problem is that local heating code always reads zeros no matter what I seem to do. Reading the created files into RRTM as an ozone does work, and reading ozone-1990 into local heating does not. Very odd. Tried everything I can think of, but we'll have to carry on testing.
…not being passed time, and therefore was not being fed to the correct interpolator within the interface type structure.
Conflicts:
	src/atmos_param/two_stream_gray_rad/two_stream_gray_rad.F90
sit23 and others added 17 commits August 4, 2026 13:38
…ase. Confirmed results bitwise identical, and have made it so that the max number of processors used for parallel compilation is 8, as this gives 32% speedup on serial compilation, but more cores does not give compilation speedup.
…oducible as random seed was not set. Now added as namelist parameter for both test cases, and verified that runs are reproducible.
…so non-Titan cases stay bit-identical to master

Both update_tracers's grid-tracer branch (spectral_dynamics.F90) and the
bucket-water diffusion call (idealized_moist_phys.F90) unconditionally ran a
grid->spherical->grid round-trip that was only meant to support new,
opt-in Titan features (do_spec_tracer_filter, damping_coeff_bucket damping).
Even with those options left at their defaults (off), the round-trip isn't a
no-op - it silently low-pass-filters real grid-scale structure (e.g. via
floating-point reassociation in the tracer tendency, or truncation of
land/ocean boundaries in the bucket case) - which broke bit-reproducibility
against master for frierson, bucket_model, and realistic_continents_variable_qflux.

Both call sites are now branched on their respective flags: the Titan path
runs unchanged when requested, and everything else falls back to the exact
pre-existing formulation. Verified bit-identical to master (eb64615) for
frierson, bucket_model, and realistic_continents_variable_qflux.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
7f4201d ("Important fix for two-stream") corrected frac_of_day to account
for total elapsed seconds rather than just the current day's remainder,
needed for solar day lengths > 86400s (Mars, Titan) where the model's
internal "days" bookkeeping (fixed 86400s calendar units) and the actual
solar day length diverge. Master never needed this fix and still uses the
original formulation.

For day_in_s == 86400 (Earth-standard), the original formulation was
already correct, but the code ran the total-elapsed-seconds path
unconditionally. Since that path needs mod() to strip the accumulated whole-days
component back out, it introduces a small precision loss vs directly using
r_seconds - breaking bit-reproducibility against master for any do_seasonal
Earth-day-length case (e.g. variable_co2_grey).

Branch on day_in_s == 86400 instead: Earth-standard runs get the exact
pre-7f4201dd formulation back, non-Earth day lengths keep 7f4201d's fix
unchanged. Verified bit-identical to master for variable_co2_grey (and
frierson, bucket_model together).

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
@sit23

sit23 commented Aug 12, 2026

Copy link
Copy Markdown
Contributor Author

Having put all of the Mars-related modifications into this P/R, nearly all of the trip tests. However, I worked with Claude to find the causes of these trip test failures, and have identified and fixed all of them. The summary of this investigation (generated by Claude) is given below:

Summary

Three genuine numerical regressions between mars_dev_gfort_local_heating_2026 and master were tracked down and fixed. All three shared the same shape: a Titan-motivated code change was added as the only code path instead of being gated behind its own (already-existing, default-off) namelist option, so it silently ran for every experiment — including ones that never asked for it — and broke bit-reproducibility against master.

  • frierson and realistic_continents_variable_qflux diverged from master (tiny for frierson, substantial for realistic_continents_variable_qflux — ps off by up to 1.7).
  • bucket_model diverged substantially (ps off by 4.5, temp by 0.5).
  • variable_co2_grey diverged moderately (ps off by 0.023).

All three are now bit-identical to master. Socrates-based trip tests (socrates_aquaplanet, socrates_aquaplanet_cloud, ape_aquaplanet), previously untestable locally due to an unrelated environment issue, now run and pass as well.

Fix 1 — spectral_dynamics.F90: grid-tracer accumulation order (commit 3cdb2e93)

update_tracers's grid-tracer branch was restructured to support a new Titan option (do_spec_tracer_filter, spectral damping of the grid tracer) by accumulating the horizontal+vertical advection tendencies into dt_tr and recomputing tr_future from it once at the end, instead of incrementing tr_future directly after each advection step. Mathematically equivalent, but a different floating-point summation order — and that changed bit-level results for every moist run, not just Titan ones, since do_spec_tracer_filter defaults to .false..

Fix: branch on do_spec_tracer_filter. The new accumulate-then-filter path runs unchanged when the option is actually requested; everything else falls back to the original direct-increment formulation, byte-for-byte.

Fix 2 — idealized_moist_phys.F90: unconditional bucket-water spectral diffusion (commit 3cdb2e93)

Same underlying pattern, bigger effect. The bucket-water tendency block gained an unconditional call to a new diffuse_surf_water() subroutine, regardless of damping_coeff_bucket (which defaults to 0., i.e. "no damping requested"). The subroutine round-trips the tendency field through a spherical harmonic transform and back — and even at zero damping, that round-trip isn't a no-op: it truncates the field to the spectral resolution, silently low-pass-filtering real grid-scale structure (e.g. land/ocean boundaries in bucket_depth). That's a genuine physical change, not roundoff, which is why bucket_model's divergence was far larger than the others.

Fix: only call diffuse_surf_water() when damping_coeff_bucket > 0.

Fix 3 — two_stream_gray_rad.F90: seasonal frac-of-day precision loss (commit ea8e155b)

Different in kind from the first two: not an accidental regression. Commit 7f4201dd ("Important fix for two-stream") correctly fixed frac_of_day to account for total elapsed seconds rather than just the current day's remainder — needed for solar day lengths > 86400s (Mars, Titan), where the model's internal "days" bookkeeping (fixed 86400s calendar units) and the actual solar day length diverge. Master never needed this and still uses the original formulation. For the standard Earth day length, the new formulation still runs and still gets the right answer, but only after mod()-ing away an accumulated whole-days component — introducing a small but real precision loss relative to just using the bounded seconds-of-day value directly. That's what broke variable_co2_grey (which sets do_seasonal: True) against master.

Fix: branch on day_in_s == 86400.. Earth-standard day length gets the exact pre-7f4201dd formulation back, bit-for-bit; non-Earth day lengths keep 7f4201dd's correction unchanged, preserving correct Mars/Titan seasonal timing.

Socrates trip tests

Previously blocked locally by an unrelated environment issue: GFDL_SOC points to a nonexistent path, and scratch codebase clones (which don't inherit the untracked trunk symlink our real checkout already has set up correctly) picked up a dangling symlink to that broken location. No code change — just needed trunk repointed at the real local Socrates source per clone, same method already in use in this checkout. All three Socrates test cases (socrates_aquaplanet, socrates_aquaplanet_cloud, ape_aquaplanet) pass against master.

Known remaining issue (not fixed in this PR)

top_down_test shows a small, stable divergence from master (ucomp ~1.2e-7, vor ~1.1e-13) that is not caused by any of the above — confirmed by reverting all candidate files from the relevant merge and finding no change. Bisecting it further is blocked by an unrelated double free or corruption crash in old commits (likely in mixed_layer.F90's heat-capacity spinup code), which would need real memory debugging rather than a mechanical fix to work around. Given this test case sees little use, it's left as the one known open item rather than pursued further here.

Test plan

  • frierson vs master: bit-identical
  • bucket_model vs master: bit-identical
  • realistic_continents_variable_qflux vs master: bit-identical
  • variable_co2_grey vs master: bit-identical
  • socrates_aquaplanet, socrates_aquaplanet_cloud, ape_aquaplanet vs master: bit-identical
  • held_suarez, MiMA, realistic_continents_fixed_sst, variable_co2_rrtm, column_test vs master: unaffected, still bit-identical
  • top_down_test vs master: still diverges (pre-existing, unrelated, documented above — not addressed in this PR)

@sit23

sit23 commented Aug 12, 2026

Copy link
Copy Markdown
Contributor Author

The plan for this branch now is to attempt to track down the cause of the top-down test divergence bug. If this can be found, a fix will be applied, and then this branch can be merged.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants