Skip to content

Md/phys tests - #31

Merged
MarijnJ0 merged 40 commits into
mainfrom
md/phys_tests
Sep 8, 2026
Merged

MarijnJ0 merged 40 commits into
mainfrom
md/phys_tests

Conversation

@MarijnJ0

@MarijnJ0 MarijnJ0 commented Aug 19, 2026

Copy link
Copy Markdown
Member

Description

This PR introduces new and improved tests, as well as several bug fixes. With these changes the solid1d* models now closely agree on tidal dissipation and Love numbers in the regime where they are stable. The test output is now visible in the docs and README through new badges. In addition to this, I have added the themes from AGNI into Obliqua, and made several changes to get the dark theme to work (it is still not perfect, but looks ok).

Specifically, this PR adds the following:

  • Added safety check for load number and scaling factor.
  • Fixed surface boundary condition used by solid1d* models.
  • Fixed sign convention between core solution / A matrix / surface boundary used by solid1d* models.
  • Fixed issue where the core solution for relaxation solvers would be assumed non-singular before computing the left over nullspace. The computation is now valid for all core solution vectors (removes rigid core rather than a free-slipping one.)
  • Added test that verifies compatibility of core and surface boundary.
  • Added test that verifies compatibility of core and A matrix.
  • Added new equilibrium tide model (Saito 1974) for inhomogeneous sphere: solid1d_equil_relax.
  • Added non-dimensional scaling to all solid1d* solvers, it is needed to determine the loading Love number as the matrix objects at the surface become rank deficient.
  • Fixed bug where first interior layer was always skipped.
  • Fixed bug where shooting method based solid1d models always skipped first layer passed to them.
  • Added tests that verify the homogeneous sphere limit, all solid1d models now closely agree with the analytical solution in solid0d. Additionally this checks that in the equilibrium limit the output converges to solid1d_equil_relax.
  • Added tests that verifies the solid core solution vector in the homogeneous limit.
  • Added option to use lowest mantle properties (shear/bulk) instead of those from the config. Usefull, for testing, or when Obliqua resolves the core and the 'core solution vector' merely acts as a guard against the singularity at r=0.
  • Obliqua now explicitely tracks all dissipation channels (shear, bulk, Darcy, friction, drag) as 3d objects.
  • fluid1d now has an updated radial distribution model that smoothly attaches to 3d dissipation objects (shear, bulk, Darcy), and extends the heating profile with isotropic friction and drag.
  • Added tests.jl badges.
  • Added light and dark themes for the docs, similar to those used by PROTEUS.

Still to add

I have already some updated plotting scripts, and will add them in the upcomming days. First I want to work on the orbit/PROTEUS part.

I want to add some additional tests for the mush models to further verify the code.

The inertial core solution vector needs to be further looked into, I have found a good source.

Let me know if I missed something! :)

Validation of changes

afbeelding afbeelding afbeelding afbeelding afbeelding afbeelding afbeelding afbeelding afbeelding

See the dissipation channel example plot in the overleaf document.

Checklist

  • I have followed the contributing guidelines
  • My code follows the style guidelines of this project
  • I have performed a self-review of my code
  • My changes generate no new warnings or errors
  • I have checked that the tests still pass on my computer
  • I have updated the docs, as appropriate
  • I have added tests for these changes, as appropriate
  • I have checked that all dependencies have been updated, as required

Relevant people

@nichollsh @timlichtenberg

@MarijnJ0
MarijnJ0 requested a review from a team as a code owner August 19, 2026 21:04
Copilot AI lite review requested due to automatic review settings August 19, 2026 21:04

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR expands Obliqua’s solid/fluid tidal response validation and updates the 1D solid (and mush) solvers to improve stability/consistency (boundary conditions, sign conventions, scaling), while also adding documentation theming and CI-visible test badges.

Changes:

  • Adds non-dimensional scaling and boundary-condition plumbing to solid1d* solvers, plus a new equilibrium-tide relaxation model (solid1d_equil_relax).
  • Adds/updates tests to validate core/surface/A-matrix compatibility and homogeneous-sphere limits across models.
  • Updates docs/README presentation (badges + Documenter light/dark themes) and CI to publish badge JSON to a gist.

Reviewed changes

Copilot reviewed 36 out of 41 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
test/test.toml Adds core_props test configuration key.
test/test_solid1d.jl Updates tests for new compute_M/compute_y signatures and complex core parameters.
test/test_solid1d_relax.jl Adjusts relax tests for complex types and updated boundary-condition logic.
test/test_solid1d_mush.jl Updates mush tests for new solver signatures and avoids r=0 in test grid.
test/test_solid1d_mush_relax.jl Updates core parameter types to complex.
test/test_solid1d_equil_relax.jl New test suite for the new equilibrium relaxation solver and analytical identities.
test/test_common.jl Adds self-consistency tests for get_Ic, get_A, and surface BC interactions; updates precision handling.
src/solid1d.jl Updates gravity computation, propagator scaling support, and compute_M/compute_y to use non-dimensional scaling and revised BCs.
src/solid1d_relax.jl Updates core BC construction (nullspace-based), types, and boundary-condition ordering.
src/solid1d_mush.jl Mirrors solid1d changes for mush case: scaling, BC updates, porous-layer handling changes.
src/solid1d_mush_relax.jl Updates core BC construction (nullspace-based) and core parameter types.
src/solid1d_equil_relax.jl Adds a new equilibrium-tide relaxation solver module (2×2 system).
src/fluid0d.jl Renames doc/variables from k2_* to kn_* in the fluid 0D Love number routine.
src/common.jl Adds shared get_surface_bc! and equilibrium-fluid get_A!/get_A; adjusts core solution conventions and heating map scaling.
res/config/all_options.toml Documents new module option and adds core_props option.
README.md Adds centered badges (license, codecov, test badges) and minor formatting tweaks.
docs/src/troubleshooting.md Adjusts heading levels.
docs/src/reference/liquid-phase.md Adjusts heading levels.
docs/src/install.md Adjusts heading levels.
docs/src/index.md Adds badges to docs landing page.
docs/src/explanation/main_loop.md Adjusts heading levels.
docs/src/development.md Adjusts heading levels and content formatting.
docs/src/assets/style.scss Adds shared SCSS palette, typography, and font-face declarations.
docs/src/assets/style.css Adds CSS overrides for badges/images and light/dark display toggles.
docs/src/assets/lightdefs.scss Adds Documenter light-theme variable overrides and custom styling.
docs/src/assets/darkdefs.scss Adds Documenter dark-theme overrides and custom CSS fixes (incl. admonitions).
docs/src/assets/light.scss Generated/combined light theme SCSS used for compilation.
docs/src/assets/dark.scss Generated/combined dark theme SCSS used for compilation.
docs/src/assets/fonts/OFL-*.txt Adds font license texts.
docs/Project.toml Adds DocumenterTools dependency for theme compilation.
docs/make.jl Compiles light/dark themes during docs build and configures HTML assets.
.github/workflows/runtests.yml Adds a step to publish test badge JSON to a gist on push.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread src/solid1d.jl Outdated
Comment thread src/solid1d_mush.jl Outdated
Comment thread .github/workflows/runtests.yml
Comment thread docs/src/development.md Outdated
@codecov

codecov Bot commented Aug 19, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 88.67188% with 87 lines in your changes missing coverage. Please review.
✅ Project coverage is 82.62%. Comparing base (a3f5650) to head (08491a8).

Files with missing lines Patch % Lines
src/Obliqua.jl 74.66% 75 Missing ⚠️
src/solid1d_mush_relax.jl 80.00% 5 Missing ⚠️
src/common.jl 99.14% 2 Missing ⚠️
src/solid1d_equil_relax.jl 97.95% 2 Missing ⚠️
src/solid1d_relax.jl 89.47% 2 Missing ⚠️
src/solid1d_mush.jl 98.27% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main      #31      +/-   ##
==========================================
+ Coverage   79.32%   82.62%   +3.30%     
==========================================
  Files          13       14       +1     
  Lines        2796     3051     +255     
==========================================
+ Hits         2218     2521     +303     
+ Misses        578      530      -48     

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

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@timlichtenberg timlichtenberg left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great progress on the tests and the theming, this is a lot of work. A few questions on the solver changes before I can approve.

get_A! in common.jl still takes ω as a required argument, but the two -ω^2*ρ inertial terms are commented out, not removed. Was that on purpose? The new Ic vs A consistency test can't catch this either way, since get_Ic's "solid" branch has no ω term.

Zero forcing frequency now reroutes to solid1d-equil-relax whenever module_solid contains "solid1d", which also matches "solid1d-mush". Doesn't that send mush segments to the plain solid-body model at σ=0, dropping the mush arguments?

The old surface condition combined y6 + (n+1)/r*y5. The new get_surface_bc! only sets y6. Is that a basis change that makes the y5 term redundant, or did it get dropped?

get_core_bc! moved from pinv to nullspace, which is the headline boundary-condition fix, but the old unit test for it (checking B's shape and entries) was deleted with nothing in its place. Worth a direct test on the new implementation?

get_g divides by r^2, including at r=0. The mush test grid moved from r=0.0 to r=0.1 in this PR. Is that working around the same division rather than fixing it?

Small one: the docs build is failing in CI. DocumenterTools is in docs/Project.toml now, but documentation.yml never installs from that project file, so docs/make.jl's using DocumenterTools errors out.

@nichollsh nichollsh left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Summary

Awesome work, @MarijnJ0 - thanks for this.

The bugfix changes to the code seem reasonable to me, although some of the sections are still light on in-line comments, which makes it hard to understand all of the linear algebra that's implemented. I assume the (many) sign-changes you've included in this PR are some of the key bug fixes in the core model? I appreciate you adding more docstrings throughout the codebase, though.

The tests are great. They seem to include a range of unit, smoke, and integration tests. You've even included comparisons against 'classic' tidal models in the appropriate endmember regimes (e.g. fully liquid tides).

I ran the tests on my laptop (Fedora 44 on AMD, Julia 1.12.6) and they all pass fine. I am able to use the coverage script which 82% coverage. The docs also build without errors and render fine.

Changes required

  1. The test count badges are created, although they are all labelled as unit tests. The integrate test badge shows zero. Is this correct?

  2. The documentation shows the same badge three times. Are these meant to be the three test-type badges? The documentation workflow is also failing because of a missing DocumenterTools package.

  3. The link to the AGNI documentation is broken in the Obliqua docs 'ecosystem' page.

  4. The reference tables explaining config parameters should go into the Reference section of the docs, not within Tutorial 2.

  5. The tutorials do not really feel like a tutorial still. They feel like a series of commands that the user runs without knowing what they do or achieve. Please at least include some figures to show what is outputted, and explain these in the tutorial pages.

  6. The "How-To guides" page is empty and the new solid1d_equil_relax model is missing a documentation entry.

  7. Please add coverage.*, src/*.cov, and docs/build/ to the .gitignore, to avoid uploading them to GitHub.

  8. See my additional in-line comments below.

Comment thread src/solid1d.jl Outdated
Comment thread src/solid1d.jl
Comment thread src/solid1d.jl
Comment thread src/solid1d_mush_relax.jl Outdated
Comment thread src/solid1d_equil_relax.jl Outdated
Comment thread src/solid1d_equil_relax.jl Outdated
Comment thread src/solid1d_relax.jl Outdated
@MarijnJ0

Copy link
Copy Markdown
Member Author

Thanks for the feedback! To answer your questions:

the two -ω^2*ρ inertial terms are commented out, not removed.

This was a typo. Since the solid0d limit is only a valid approximation of solid1d when the inertial terms are removed from the motion matrix, I had commented them out during testing. I added the inertial_terms option to enable/disable inertial terms in motion matrix. This is useful for the shooting method, as it tends to diverge more quickly with these terms enabled. The relaxation based solver handles them no problem.

Doesn't that send mush segments to the plain solid-body model at σ=0,

No, the solid1d_mush models are simply solid tide models that also capture the Darcy dissipation channel, which is only active in partial melts (mush). As such, using the solid1d_mush models requires the user to merge the solid and mush regimes down to some user defined viscosity. When using the relaxation based solver we can merge the entire solid and mush regimes. Regardless of the choice, the distinct tidal dissipation channels all turn off in the equilibrium limit, hence Im(k_n) = 0 by definition. The deformation is purely elastic, and the Re(k_n) depends merely on the mass distribution (i.e. the density profile). This means that solid1d_equil_relax is the correct simplified version of all other solid1d models. I suppose that if we drop the assumption of homogenity in the fluid regime, then the equations would also reduce to solid1d_equil_relax, Potentially we should apply solid1d_equil_relax in the entire mantle, but we would be mixing assumptions at this point, hence I only added it in place of the solid1d (inhomogeneous) models.

The old surface condition combined y6 + (n+1)/r*y5.

This is because of a mistake from before. I had originally confused two conventions. We should not include y5 in the surface boundary given our definition of y6 (see overleaf). This was introduced in the more recent works to simplify the surface boundary conditions. Additionally, I changed the normalization of the loading potential (G/G0 / R * U_prime). The latter does not apply to the equilibrium model, but since it relies on y7 = y6 + 4πG/g y2 it is always zero, no matter the normalization. With these changes, the boundary conditions now match those used in Sabadini 2016.

Worth a direct test on the new implementation?

I added a test that checks if B is the nullspace through B * Ic = 0.

Is that working around the same division rather than fixing it?

The solid1d models all have a singularity at r=0, not just due to the gravity calculation, but also in the motion matrices. Hence the analytically derived core solution vectors. They literally allow you to set r as small as you like > 0 (you dont have to use them to 'model' the entire metal core, we just do so because we dont have the inner core radial profiles in obliqua yet). So that, say the first layer is set to start at r = 0, then we must at least set r --> r = 0 + epsilon for some tiny epsilon. Since the first layer skip bugs got fixed I had to change the r=0 to r=0.1. In the main run_tides function I now added a check that sets r[1] --> 1e-6 * r[2] if r[1] = 0. As well as, several checks in the run_solid wrapper functions to catch this.

DocumenterTools

I added it to the build script, it should work now.

integrate test badge shows zero. Is this correct?

Yes, I did not add these tests yet. But copied over the badges etc, so that it will automatically grab these once they are added.

Note, I changed the runtests.yml file temporarly, but will add back the if statement:

if: ${{(github.event_name == 'push') && (github.ref == 'refs/heads/main')}}

same badge three times. Are these meant to be the three test-type badges?

Yes they should be different: unit, integ, unit+integ.

The link to the AGNI documentation is broken in the Obliqua docs 'ecosystem' page.

I have changed it to the one used on the PROTEUS site: https://www.h-nicholls.space/AGNI/ (please lmk if this is incorrect.). Additionally, I changed the rest to the proteus-framework.org/ urls.

config parameters should go into the Reference

Moved the description to the reference, kept the read command in the tutorial.

Please at least include some figures to show what is outputted, and explain these in the tutorial pages.

The "How-To guides" page is empty

This will take some time, I will try improving the docs over the next few days.

the new solid1d_equil_relax model is missing a documentation entry.

Added a new page for this.

Please add coverage., src/.cov, and docs/build/ to the .gitignore

Done.

I will wait for the tests to finish, and see if the badges get updated...

@MarijnJ0

Copy link
Copy Markdown
Member Author

Docs preview at https://proteus-framework.org/Obliqua/previews/PR31/

The badges where created and look correct. I'll change the runtests.yml back now.

@nichollsh nichollsh left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for this Marijn. I love the animation on the homepage! Great work. However, I have a number of critical suggestions which need to be addressed before this is ready. See my in-line comments below and higher-level comments in this review.

This is my "human" review, but I'll get Claude to take a pass, too.

Tests

The tests pass on my laptop (Fedora 44 on x86, Julia 1.12.6). The coverage is 81.7% - which is very nice.

Tutorial 0D

I managed to get the tutorial working, but it doesn't work as-described in the docs. The final plot looks as below, which does indeed reproduce the one you've provided.

Image

Tutorial 1D

Doesn't work because the file channel_showcase_updated.json is missing from the repo. I tried using the test_*.json files instead, but they fail with a KeyError on "phi".

Write NetCDF error

When writing the output netcdf files, I get the following error if the file already exists. Consider checking + warning + removing the file first, if it exists.

``log
[ Info: Writing results to /home/harrison/Projects/Obliqua/out/0_obliqua.nc
ERROR: NetCDF error: Permission denied (NetCDF error code: 13)


### Docs links

Many of the documentation links are broken. This is for various reasons including: incorrect HTML/Markdown syntax, linking directly to proteus-framework.org, typos, and some links are simply missing.

For example, the page `running_output.md` doesn't appear in the sidebar.

Comment thread docs/src/index.md Outdated
Comment thread docs/src/index.md Outdated
Comment thread docs/src/index.md Outdated
Comment thread docs/make.jl
Comment thread docs/src/index.md
Comment thread docs/src/get_started.md Outdated
Comment thread docs/src/how-to-guides/config_file.md Outdated
Comment thread .gitignore
Comment thread docs/src/tutorials/0d_test.md
Comment thread docs/src/tutorials/0d_test.md
@nichollsh

nichollsh commented Sep 2, 2026

Copy link
Copy Markdown
Member

Selected comments from Claude code review, two things:

  1. Undiscussed sign change in an untested fluid-body matrix — src/common.jl:1044

The 4x4 fluid-body get_A!(A, ω, r, ρ, g, K, n; ...) (docstring cites Korenaga (2025) Eq. 12) changed:

A[Y[2],Y[3]] = ρ*(n+1)*r_inv + n*(n+1)*ρ*g / (r^2 * ω^2)

to

A[Y[2],Y[3]] = ρ*(n+1)*r_inv - n*(n+1)*ρ*g / (r^2 * ω^2)

Unlike the other sign changes in this PR, this one isn't mentioned anywhere. It also has no callers anywhere in src/ and is never exercised by any test — confirmed via the .cov coverage annotations, where this function's lines show no hit count at all while neighboring functions show large hit counts. A sign error here would never be caught by CI. Either add a test that pins it against Korenaga Eq. 12 (or against the same Ic-vs-A finite-difference consistency check used elsewhere in test_common.jl, if a matching Ic exists), or remove it if it's vestigial.

  1. solid_shell patch leaks its placeholder moduli into the heating-map output — src/solid1d_relax.jl:61-65, src/solid1d_mush_relax.jl:72-78

When patch=true (set in Obliqua.jl whenever solid_shell is on and the segment is solid-relax/mush-relax with all layer viscosities ≤ 1e17 Pa·s), solve_radial_system does:

μ[1] = precc(1.47e11)
K[1] = precc(6.58e10)

directly on its μ/K arguments — an in-place mutation, not a copy. In Obliqua.jl, run_solid1d_relax and run_solid1d_mush_relax pass the very same μc/κc arrays into get_heating_profile/get_heating_map right after compute_y returns. Since the patch is never undone, the heating diagnostic at the patched (core-adjacent) layer is silently computed with these placeholder moduli instead of the real layer material, with nothing documenting that this happens. (It doesn't leak across time-steps — resample_profiles/copy() isolate the array per call — but it does corrupt that call's own heating output.)

Suggest operating on a local copy of μ/K (and Kd/k/ϕ in the mush case) inside the patch branch, or computing the heating profile/map from the un-patched moduli explicitly, or — if the patched-layer heating is actually meant to reflect the placeholder shell — documenting that explicitly.

@MarijnJ0

MarijnJ0 commented Sep 2, 2026

Copy link
Copy Markdown
Member Author

Thanks Harrison!

I have added the requested fixes to the docs. Regarding the code, the unused 4x4 A matrix was added mainly for testing purposes, but since it doesn't add much to the relaxation based solvers, I decided to remove it completely. Potentially, it can be reintroduced at some point in the future for completeness.

The issue with the patch leaking into the heating profile is now also fixed. Before reassigning the first entry in the array, the code now first creates a local copy.

I tried both tutorials and they seem to work, I added the new data file so this should also be fixed now.

For plotting I kept the python notebook, as I think it is more convenient to work with than plots.jl. For this, there is now also a conda install section in the docs.

If you have any further comments do let me know!

@nichollsh nichollsh left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great - thank you, @MarijnJ0, for implementing my suggestions and fixes.

This seems to work as described on my laptop. The tests still pass with good code coverage.

The 1D tutorial now works, and I get the plot below, which matches the documentation.

Image

I will mention that the example notebook contains some "empty" plots of heating vs lon/lat. This isn't mentioned in the tutorial at the moment, so I suppose they are remnants or intended for future expansion?

Happy for this to be merged. Well done!

@nichollsh

Copy link
Copy Markdown
Member

This PR is currently blocked by @timlichtenberg's previous review requesting changes. I have given it a look, but @timlichtenberg needs to re-review or dismiss the old one before it can be merged.

@MarijnJ0

MarijnJ0 commented Sep 3, 2026

Copy link
Copy Markdown
Member Author

Thanks again, great to hear it is working!

FYI: The empty plots for the heatmaps only get populated when store_3D = true, otherwise the angular grid gets averaged over the solid angle, and we only end up with the heating profile.

@MarijnJ0

MarijnJ0 commented Sep 6, 2026

Copy link
Copy Markdown
Member Author

These two commits also add the relevant fixes for the solid1d_mush_relax model and the inertial core solution vector.

I verified the mush model against the description in overleaf, and they match. The final part of the derivation is now in overleaf, and is more of a simplification step.

Additionally, to fix the inertial core solution vector, I found a typo in the original reference: https://www.aanda.org/articles/aa/full_html/2021/06/aa39433-20/aa39433-20.html see inline comments. This now adds the fully general solid solution, verified against A through finite-difference checks. This simplifies in the fluid limit to the inertial-liquid solution (https://www.sciencedirect.com/science/article/pii/S0019103525003045), which was originally called inertial. This is also verified against A through finite-difference checks.

The core solution vectors relate roughly as follows:

  • "inertial" reduces to "inertial-liquid" as μ -> 0: an oscillating
    solid without shear rigidity is an oscillating fluid.
  • "inertial" reduces to "solid" as ω -> 0 and K -> infinity: the
    static, incompressible limit of the general oscillating solid recovers
    the classical elastic solution.
  • "solid" (μ -> 0) reduces to "liquid" (ω = 0): incompressible, static
    elasticity without shear rigidity is a hydrostatic fluid.
  • "inertial-liquid" reduces to "liquid" as ω -> 0: a slowly
    oscillating fluid approaches hydrostatic equilibrium.

@timlichtenberg timlichtenberg left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is much improved. The new validation layer is exactly the kind of check I wanted to see: the core basis vectors are now checked against the motion matrix by finite-difference residual and against the Love 1911 closed form, and the 1D solid modules are checked against the 0D homogeneous-sphere solution in the analytical limit. That moves the physics from self-consistent to validated against independent references.

I ran the full suite on md/phys_tests (Julia 1.12): 591 tests pass, 0 failures.

Good work improving the PR. Approving.

@MarijnJ0
MarijnJ0 merged commit 570161b into main Sep 8, 2026
4 checks passed
@MarijnJ0
MarijnJ0 deleted the md/phys_tests branch September 8, 2026 14:42
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.

Extension of tests across PROTEUS ecosystem

4 participants