Skip to content

oceanic plate feature with half space cooled topography model#908

Open
simransuresh wants to merge 10 commits into
GeodynamicWorldBuilder:mainfrom
simransuresh:ocean_topo
Open

oceanic plate feature with half space cooled topography model#908
simransuresh wants to merge 10 commits into
GeodynamicWorldBuilder:mainfrom
simransuresh:ocean_topo

Conversation

@simransuresh
Copy link
Copy Markdown
Contributor

@simransuresh simransuresh commented Mar 28, 2026

  • half space cooling temperature model with age as outcome (Turcotte and Schubert book)
  • age plugged into the initial topography of the model.

@coveralls
Copy link
Copy Markdown

coveralls commented Mar 28, 2026

Pull Request Test Coverage Report for Build 23848703918

Warning: This coverage report may be inaccurate.

This pull request's base commit is no longer the HEAD commit of its target branch. This means it includes changes from outside the original pull request, including, potentially, unrelated coverage changes.

Details

  • 74 of 75 (98.67%) changed or added relevant lines in 1 file are covered.
  • 31 unchanged lines in 8 files lost coverage.
  • Overall coverage increased (+0.08%) to 98.469%

Changes Missing Coverage Covered Lines Changed/Added Lines %
source/world_builder/features/oceanic_plate_models/topography/half_space_cooling.cc 74 75 98.67%
Files with Coverage Reduction New Missed Lines %
source/world_builder/features/continental_plate_models/density/uniform.cc 1 97.56%
source/world_builder/features/fault_models/density/uniform.cc 1 96.97%
source/world_builder/features/mantle_layer_models/density/uniform.cc 1 97.56%
source/world_builder/features/oceanic_plate_models/density/uniform.cc 1 97.56%
source/world_builder/features/plume_models/density/uniform.cc 1 96.97%
source/world_builder/features/subducting_plate_models/density/uniform.cc 1 96.97%
source/world_builder/world.cc 5 92.51%
source/world_builder/parameters.cc 20 97.12%
Totals Coverage Status
Change from base Build 23748207793: 0.08%
Covered Lines: 11188
Relevant Lines: 11362

💛 - Coveralls

@github-actions
Copy link
Copy Markdown

github-actions Bot commented Mar 29, 2026

Benchmark Main Feature Difference (99.9% CI)
Slab interpolation simple none 1.313 ± 0.009 (s=341) 1.317 ± 0.017 (s=346) +0.0% .. +0.5%
Slab interpolation curved simple none 1.315 ± 0.008 (s=361) 1.321 ± 0.007 (s=324) +0.3% .. +0.6%
Spherical slab interpolation simple none 1.284 ± 0.009 (s=333) 1.292 ± 0.014 (s=368) +0.4% .. +0.9%
Slab interpolation simple curved CMS 1.223 ± 0.011 (s=358) 1.226 ± 0.015 (s=379) -0.0% .. +0.5%
Spherical slab interpolation simple CMS 1.625 ± 0.010 (s=279) 1.636 ± 0.010 (s=276) +0.5% .. +0.8%
Spherical fault interpolation simple none 1.289 ± 0.006 (s=341) 1.299 ± 0.007 (s=357) +0.7% .. +0.9%
Cartesian min max surface 3.129 ± 0.022 (s=129) 3.136 ± 0.023 (s=161) -0.0% .. +0.5%
Spherical min max surface 7.870 ± 0.049 (s=61) 7.896 ± 0.051 (s=56) -0.1% .. +0.7%

@simransuresh
Copy link
Copy Markdown
Contributor Author

image

Ridge center at x=100e3 on a total dimension of x=1150e3. Topography decreases from the ridge center towards the plate width.

@danieldouglas92
Copy link
Copy Markdown
Contributor

@simransuresh That picture is very cool and I'm very excited to try this out in my subduction models! I can only imagine this will reduce the amount of computation time spent isostatically adjusting the subducting plate. Out of curiosity, have you compared what the topography is along a ridge perpendicular profile to what the analytical solution should be?

I'll review your actual code soon.

Copy link
Copy Markdown
Contributor

@danieldouglas92 danieldouglas92 left a comment

Choose a reason for hiding this comment

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

Hey @simransuresh

Thanks for this, as I mentioned already I'm excited to try this feature in my own subduction models. I only had two minor comments on your code

Copy link
Copy Markdown
Contributor

@danieldouglas92 danieldouglas92 left a comment

Choose a reason for hiding this comment

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

Thanks for addressing my comments @simransuresh! Sorry I missed this before, but please change the name of the class to follow the camel case standard used elsewhere in the world builder. Also, did you see my previous comment about plotting the topography against the solution for subsidence with age (i.e. from Turcotte & Schubert)?

@simransuresh
Copy link
Copy Markdown
Contributor Author

simransuresh commented Mar 31, 2026

Thanks for addressing my comments @simransuresh! Sorry I missed this before, but please change the name of the class to follow the camel case standard used elsewhere in the world builder. Also, did you see my previous comment about plotting the topography against the solution for subsidence with age (i.e. from Turcotte & Schubert)?

image

sorry I missed to add it! Here it is, I hope this is what you asked for - topo vs plate age

@simransuresh
Copy link
Copy Markdown
Contributor Author

Even with "make indent", my tests are still failing :( I am trying to debug..

@MFraters
Copy link
Copy Markdown
Member

you checked in your build directory. please remove it from your commits. That that should fix the indenter

const double alpha = this->world->thermal_expansion_coefficient;

// age of half space cooling model is used to compute the heights and added with initial depth and topo
double half_space_cooling_height = (2.0 * bottom_density * alpha * (bottom_temperature - top_temperature) / top_density) *
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Could you point me to the resource where you get this? This is what I am looking at from Turcotte and Schubert:

Image

which means that the denominator you have looks different or maybe you have a different interpretation for top_density?

Copy link
Copy Markdown
Contributor Author

@simransuresh simransuresh Apr 1, 2026

Choose a reason for hiding this comment

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

Yes you're right, this is the formula i referred to (first commit in this PR). But when adding the top density as input param I missed to add the "bottom density -" too in the denom. Apology for the that. I will recommit with the correct formula.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

No need for apology, we all make mistakes and that's what reviews are for ;)

@simransuresh
Copy link
Copy Markdown
Contributor Author

@alarshi and @danieldouglas92 I have addressed all your comments.

Copy link
Copy Markdown
Contributor

@danieldouglas92 danieldouglas92 left a comment

Choose a reason for hiding this comment

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

@simransuresh If the new formula you added reproduces the expected topography from turcotte & schubert then I think your code looks good to me! Just one quick fix with changing "cooled" to "cooling" in the doc string for the class.

Three other quick things:

  1. Don't forget to add an entry to the changelog!
  2. Please remove world_builder_declarations.tex from your committed files
  3. Please squash your commits down to just a couple.

If @alarshi agrees, once you've done this I think this is good to merge! Thanks again for adding this I think this is really great

prm.declare_entry("", Types::Object({"ridge coordinates", "spreading velocity",
"max depth", "min ocean depth", "bottom density",
"top density"
}), "Half space cooled topography");
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggested change
}), "Half space cooled topography");
}), "Half space cooling topography");

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.

5 participants