Skip to content

Feature-level composition parser#934

Open
alanjyu wants to merge 3 commits into
GeodynamicWorldBuilder:mainfrom
alanjyu:feature_composition_parser
Open

Feature-level composition parser#934
alanjyu wants to merge 3 commits into
GeodynamicWorldBuilder:mainfrom
alanjyu:feature_composition_parser

Conversation

@alanjyu
Copy link
Copy Markdown
Contributor

@alanjyu alanjyu commented Mar 31, 2026

Following #909, #926 (obsolete), and #932, users now can assign compositions using an array of names (string) or indices (unsigned int) that they define in the composition_properties map. Name-based entries are parsed and resolved to their corresponding composition indices (and associated default properties)

Although the schema and tests are currently valid, I have not yet identified edge cases where parsing may fail for random composition profiles. In addition, the output naming function has not been updated yet, so visualization still shows Composition_[n] labels.

@danieldouglas92
Copy link
Copy Markdown
Contributor

Thanks for breaking up #926, this makes the current new functionality you're adding here much easier to digest

check_entry(const std::string &name) const;

struct composition_property
{
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Moved up since the struct must be declared before get_vector()

"name":"TestPlate",
"max depth":300e3,
"coordinates":[[0,0],[100e3,0],[100e3,100e3],[0,100e3]],
"coordinates":[[0,0],[100e3,0],[100e3,50e3],[0,50e3]],
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Add a test case for different plate models

@coveralls
Copy link
Copy Markdown

coveralls commented Mar 31, 2026

Coverage Report for CI Build 24134920095

Warning

Build has drifted: This PR's base is out of sync with its target branch, so coverage data may include unrelated changes.
Quick fix: rebase this PR. Learn more →

Coverage decreased (-0.2%) to 98.245%

Details

  • Coverage decreased (-0.2%) from the base build.
  • Patch coverage: 7 uncovered changes across 1 file (96 of 103 lines covered, 93.2%).
  • No coverage regressions found.

Uncovered Changes

File Changed Covered %
source/world_builder/parameters.cc 31 24 77.42%

Coverage Regressions

No coverage regressions found.


Coverage Stats

Coverage Status
Relevant Lines: 11969
Covered Lines: 11759
Line Coverage: 98.25%
Coverage Strength: 547504.82 hits per line

💛 - Coveralls

@github-actions
Copy link
Copy Markdown

github-actions Bot commented Mar 31, 2026

Benchmark Main Feature Difference (99.9% CI)
Slab interpolation simple none 1.298 ± 0.006 (s=337) 1.279 ± 0.006 (s=364) -1.6% .. -1.3%
Slab interpolation curved simple none 1.305 ± 0.008 (s=354) 1.283 ± 0.017 (s=344) -1.9% .. -1.4%
Spherical slab interpolation simple none 1.275 ± 0.008 (s=364) 1.258 ± 0.010 (s=349) -1.5% .. -1.1%
Slab interpolation simple curved CMS 1.339 ± 0.007 (s=325) 1.318 ± 0.007 (s=355) -1.7% .. -1.5%
Spherical slab interpolation simple CMS 1.559 ± 0.013 (s=290) 1.578 ± 0.015 (s=286) +1.0% .. +1.5%
Spherical fault interpolation simple none 1.289 ± 0.007 (s=344) 1.273 ± 0.010 (s=361) -1.4% .. -1.1%
Cartesian min max surface 2.842 ± 0.022 (s=170) 2.896 ± 0.028 (s=146) +1.6% .. +2.2%
Spherical min max surface 7.991 ± 0.091 (s=57) 8.152 ± 0.091 (s=57) +1.3% .. +2.7%

@MFraters
Copy link
Copy Markdown
Member

Can you split out the fix typos into a seperate pull request (make a new branch from main, and you can use git cherry-pick commit hash). That make reviewing this pull request easier :)

@alanjyu alanjyu force-pushed the feature_composition_parser branch from b7ae84f to a12a8c6 Compare March 31, 2026 16:51
@alanjyu
Copy link
Copy Markdown
Contributor Author

alanjyu commented Mar 31, 2026

I squashed my own typos into Extend the parser to all features. The typo in the installation_methods.md is left as is.

Maybe I should drop the Git GUI completely :).

@alanjyu
Copy link
Copy Markdown
Contributor Author

alanjyu commented Mar 31, 2026

On my machine with Windows Subsystem for Linux it compiles just fine, can't figure out why it fails some tests here. I am afraid to keep pushing commits just to test.

@MFraters
Copy link
Copy Markdown
Member

On my machine with Windows Subsystem for Linux it compiles just fine, can't figure out why it fails some tests here. I am afraid to keep pushing commits just to test.

Don't be afraid, push as much as you need to find and fix the issue. The testers automatically stop and restart if you push new commits.

I think the problem is that you need to include world.h (or where ever composition_properties is defined) into uniform.cc, but I haven't tested it.

@MFraters
Copy link
Copy Markdown
Member

You might be able to reproduce the issue if you turn off the unity build. You can do that with ccmake . in your build folder. You will get a tui (terminal ui) and you can set WB_UNITY_BUILD to OFF by navigating to it and pressing enter. Then use c to configure and g to generate. After that try recompiling again.

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.

4 participants