Add indicator field by lhy11009 · Pull Request #886 · GeodynamicWorldBuilder/WorldBuilder

lhy11009 · 2026-03-26T22:57:47Z

In the PR, I introduce a new field "indicator". In the example, an indicator field is derived from the depth of points in a feature of an oceanic plate.

@mibillen Here is how I am going to connect the prescribed solution to GWB.

lhy11009 · 2026-03-26T22:59:46Z

I include a test of this new field.

Here is the test layout:
There are two oceanic plates of 250 km depth on the surface of the model domain (they have slightly different T).

The indicator field is assigned to the plate to the left, and between depths of 50 and 100 km:

lhy11009 · 2026-03-26T23:04:55Z

Currently, this is only added to the oceanic plate, so a lot of the tests will fail, because this field is required in the main function of gwb-grid.

lhy11009 · 2026-03-26T23:05:59Z

I also haven't modified the gwb-dat function yet.

@MFraters, please let me know if this is the right way to move forward.

I think in order to make this work, we essentially need to add this field and models to every feature.

lhy11009 · 2026-03-27T19:30:56Z

Following @MFraters's suggestion. I modelled this like the composition field to have a list of entry.
Currently, I hard-code the length of the list to 3. The index 0, 1, 2 would correlate to "temperature", "velocity", "composition", respectively.

In usage, they don't specify which component of velocity or which component of composition I am going to prescribe, but only indicate I am going to prescribe these fields in general. I would incorporate this additional information as inputs to ASPECT instead.

I also decided to move the number of the entry later (i.e. 7 to 8), after Derek's PR with another new field is merged. This way, it would be easier to test the feature for now.

lhy11009 · 2026-03-27T19:32:23Z

This is the layout of the test after I modified the indicator to a list of entries. Indicators 0 and 1 are assigned, while indicator 2 is not.

alarshi

@lhy11009 : Thank you for the contribution! I just looked at your changes in the features folder and added my comments. I will take another look after I understand your goal more clearly. I have two comments that might help me in this process:

Have you tested your path with multiple indicator models? For example, if you include another indicator model in "second oceanic plate" that only defines indicator for velocity and have some overlap in the geometries of the two, are the indicators for temperature and velocity as what you would expect? I don't fully understand how the operation works in your case.
Since your goal is to identify regions within your wb feature geometries that can then use some prescribed values from ASPECT, would it be possible to achieve this by assigning some nonphysical values at those locations (say, nan) and then checking for them in ASPECT ? I guess nan is not an option because that might trigger several assertions, but you get the idea. My comment is arising from not being able to fully understand how to use this property, so feel free to ignore it if it doesn't make sense.

alarshi · 2026-03-28T02:09:17Z

+          // The type needs to be stored in a separate value, otherwise there are memory issues
+          const Types::String type = Types::String("replace", std::vector<std::string> {"replace", "add", "subtract"});
+
+          prm.declare_model_entries("temperature",parent_name, get_declare_map(),required_entries,
+          {
+            std::tuple<std::string,const WorldBuilder::Types::Interface &, std::string>
+            {
+              "operation", type,
+              "Whether the value should replace any value previously defined at this location (replace), "
+              "add the value to the previously define value (add) or subtract the value to the previously "
+              "define value (subtract)."
+            }
+          });


This code block is leftover from a copy. I don't think you need the operation for what you are trying to achieve, right? I am just thinking whether there would be a scenario where you would have multiple indicator models and you would want to combine them in some way. Therefore, I would replace this with: prm.declare_model_entries("indicator",parent_name, get_declare_map(),required_entries); But, if you think otherwise, let me know.

If I were to delete these, I would get the following error:

[lochy@lochy-shilofue1 oceanic_plate_indicator]$ /home/lochy/Softwares/WorldBuilder/build/bin/gwb-grid oceanic_plate_indicator.wb oceanic_plate_indicator.grid
Could not start the World Builder from file 'oceanic_plate_indicator.wb', error: AssertThrow value != nullptr failed in /home/lochy/Softwares/WorldBuilder/source/world_builder/parameters.cc at line 244: internal error: could not retrieve the default value at: /features/0/indicator models/0/operation/default value

Can you suggest a fix to this? I am actually not sure how this interface works and why this is called in the plate_model.cc like this:

prm.declare_entry("indicator models", Types::PluginSystem("", Features::OceanicPlateModels::Indicator::Interface::declare_entries, {"model"}),

alarshi · 2026-03-28T03:28:44Z

+          operation(Operations::REPLACE)
+        {
+          this->world = world_;
+          this->name = "depth range";


I am curious why you chose this name. I am looking similarly implemented properties, temperature and composition, which use uniform to describe what you want to do with indicator model here, and it might just preserve some uniformity ;)

I am not opposed to the name if "uniform", but would still support "depth range". This name better describes the usage case of this. Where the prescribed condition is by "depth range".

While I see your point @lhy11009, I agree with @alarshi, all worldbuilder features are specified within a depth range so this name does not really say what you are doing to the return field within that depth range. What the name of each .cc file represents to me, is what you set the value to WITHIN the desired depth range (i.e. a uniform value, or a linear profile, or a half space cooling profile etc.). Since it also preserves uniformity, my vote would be to change the name to uniform.

alarshi · 2026-03-28T04:00:54Z

+                  for (unsigned int i =0; i < indicators.size(); ++i)
+                    {
+                      const double new_indicator = 1.0;
+                      if (i == indicator_number)


I am confused how that would work if your indicator_number is 2 and size is 1, see my comment in the PR.

Good catch! I think it should be:

indicator[i] == indicator_number

tjhei

I am not sure I understand what this does and how this is different than a composition. Can you explain what you are trying to achieve?

tjhei · 2026-03-29T18:52:11Z

+      {
+        class ObjectFactory;
+
+        class Interface


Documents are before the "namespace" in this part. I think this seems to be the convention of WB file as in other places.

lhy11009 · 2026-03-30T00:45:36Z

I am not sure I understand what this does and how this is different than a composition. Can you explain what you are trying to achieve?

This is to have a separate indicator field that can indicate, say, a region in ASPECT that I could prescribe the solution of temperature, velocity, and composition from the WorldBuilder.

lhy11009 · 2026-03-30T01:21:36Z

@lhy11009 : Thank you for the contribution! I just looked at your changes in the features folder and added my comments. I will take another look after I understand your goal more clearly. I have two comments that might help me in this process:

Have you tested your path with multiple indicator models? For example, if you include another indicator model in "second oceanic plate" that only defines indicator for velocity and have some overlap in the geometries of the two, are the indicators for temperature and velocity as what you would expect? I don't fully understand how the operation works in your case.

Since your goal is to identify regions within your wb feature geometries that can then use some prescribed values from ASPECT, would it be possible to achieve this by assigning some nonphysical values at those locations (say, nan) and then checking for them in ASPECT ? I guess nan is not an option because that might trigger several assertions, but you get the idea. My comment is arising from not being able to fully understand how to use this property, so feel free to ignore it if it doesn't make sense.

Here I made this new test with two plate overlapping. The first plate spans 0 - 500 km in x, while the second plate spans 300 - 1000 km in x. So the second plate should replace the first plate in the range of 300 - 500 km. And for the second plate I assigned indicator 2. And the results are listed:

Indicator 0

Indicator 1

Indicator 2

lhy11009 · 2026-03-30T01:33:41Z

+                }
+            }
+          else
+            // else if (depth <= feature_max_depth && depth >= feature_min_depth)


@MFraters, I am actually surprised that this "else if" condition would behave differently from the "else" condition and doesn't give me what I want. Can you tell me why that is the case?

Here, I want to enforce zero value if it's outside of the assigned depth range but still inside the feature depth range.

I think we'll discuss this question between us before moving further. No worry, I keep tracking a few things that I'll discuss with you.

lhy11009 · 2026-03-30T01:34:46Z

@MFraters, can you take a look at this piece and some of the unresolved conversations before I duplicate it to other features?

lhy11009 · 2026-03-31T00:07:55Z

As @MFraters recommended, I added "indicator properties" to the parameters and used names of "temperature", "velocity", and "composition" to represent entries in the wb file.
I added a unit-test for this feature.
The two gwb-grid tests show usage cases of this new implementation. There are also leftover questions in conversations as well. Feel free to take a look.

@alarshi Feel free to take a second look. I really appreciate the time you guys spent.

lhy11009 · 2026-03-31T00:10:22Z

+      /**
+       * A map from indicator index to its properties for quick lookups.
+       */
+      std::map<unsigned int, Parameters::indicator_property> indicator_properties;


I modeled this after the composition_properties, but actually, I don't quite get it why it has to be a map object. Would it be sufficient to go with just Parameters::indicator_property.

I originally implemented this as a map, so when users define a composition with a random index, the composition can still be mapped to the global properties table. I experimented with storing it as a vector, but supporting arbitrary indices becomes more convoluted with vectors. "composition_property" (struct) stores all compositional properties for one composition, and "composition_properties" (map) indicates the table for all compositions.

Yeah, this makes sense to me. Thanks for the explanation. Then having your example in these other PRs is very helpful to implement the current structure.

coveralls · 2026-03-31T00:16:57Z

Pull Request Test Coverage Report for Build 23773804128

Details

170 of 189 (89.95%) changed or added relevant lines in 7 files are covered.
No unchanged relevant lines lost coverage.
Overall coverage decreased (-0.1%) to 98.344%

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
source/gwb-grid/main.cc	13	14	92.86%
source/world_builder/types/indicator_property.cc	22	23	95.65%
source/world_builder/features/oceanic_plate_models/indicator/depth_range.cc	42	44	95.45%
source/world_builder/world.cc	11	18	61.11%
source/world_builder/parameters.cc	48	56	85.71%

Totals
Change from base Build 23770181351:	-0.1%
Covered Lines:	11284
Relevant Lines:	11474

💛 - Coveralls

github-actions · 2026-03-31T00:22:48Z

Benchmark	Main	Feature	Difference (99.9% CI)
Slab interpolation simple none	1.307 ± 0.006 (s=340)	1.308 ± 0.006 (s=351)	-0.0% .. +0.2%
Slab interpolation curved simple none	1.309 ± 0.007 (s=338)	1.314 ± 0.011 (s=351)	+0.2% .. +0.6%
Spherical slab interpolation simple none	1.157 ± 0.006 (s=386)	1.161 ± 0.006 (s=393)	+0.2% .. +0.5%
Slab interpolation simple curved CMS	1.346 ± 0.005 (s=333)	1.349 ± 0.011 (s=337)	+0.0% .. +0.4%
Spherical slab interpolation simple CMS	1.626 ± 0.007 (s=285)	1.630 ± 0.010 (s=270)	+0.0% .. +0.4%
Spherical fault interpolation simple none	1.297 ± 0.008 (s=368)	1.298 ± 0.010 (s=328)	-0.2% .. +0.2%
Cartesian min max surface	2.841 ± 0.034 (s=167)	2.839 ± 0.027 (s=152)	-0.5% .. +0.3%
Spherical min max surface	8.644 ± 0.110 (s=50)	8.646 ± 0.094 (s=57)	-0.8% .. +0.8%

lhy11009 · 2026-03-31T00:29:40Z

+  template<>
+  std::vector<unsigned int>
+  Parameters::get_vector(const std::string &name,
+                         const std::map<unsigned int, Parameters::indicator_property> &indicator_properties)


Related to the last question, this get_vector function is a little bit complex. I tried to model from the unmerged PR and get it to work. But I am not sure whether my implementation is good enough.

I think we can come back to this after #926 is merged.

lhy11009 commented Mar 26, 2026

View reviewed changes

Comment thread tests/gwb-grid/oceanic_plate_indicator.wb

lhy11009 commented Mar 26, 2026

View reviewed changes

Comment thread source/gwb-grid/main.cc Outdated

lhy11009 closed this Mar 26, 2026

lhy11009 reopened this Mar 26, 2026

MFraters added hackathon-2026 labels Mar 27, 2026

lhy11009 force-pushed the indicator_field branch from cde2f8a to 5db2e31 Compare March 27, 2026 18:01

alarshi reviewed Mar 28, 2026

View reviewed changes

tjhei reviewed Mar 29, 2026

View reviewed changes

lhy11009 force-pushed the indicator_field branch from 5db2e31 to 97a0aba Compare March 30, 2026 01:30

lhy11009 commented Mar 30, 2026

View reviewed changes

Add indicator field

a012b8c

lhy11009 force-pushed the indicator_field branch from 97a0aba to a012b8c Compare March 31, 2026 00:04

lhy11009 commented Mar 31, 2026

View reviewed changes

Conversation

lhy11009 commented Mar 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

lhy11009 commented Mar 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

lhy11009 commented Mar 26, 2026

Uh oh!

Uh oh!

lhy11009 commented Mar 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

lhy11009 commented Mar 27, 2026

Uh oh!

lhy11009 commented Mar 27, 2026

Uh oh!

alarshi left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lhy11009 Mar 29, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

danieldouglas92 Mar 30, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tjhei left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

lhy11009 commented Mar 30, 2026

Uh oh!

lhy11009 commented Mar 30, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lhy11009 commented Mar 30, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

lhy11009 commented Mar 31, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

alanjyu Mar 31, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

coveralls commented Mar 31, 2026

Pull Request Test Coverage Report for Build 23773804128

Details

💛 - Coveralls

Uh oh!

github-actions Bot commented Mar 31, 2026

Uh oh!

lhy11009 commented Mar 26, 2026 •

edited

Loading

lhy11009 commented Mar 26, 2026 •

edited

Loading

lhy11009 commented Mar 26, 2026 •

edited

Loading

lhy11009 Mar 29, 2026 •

edited

Loading

danieldouglas92 Mar 30, 2026 •

edited

Loading

lhy11009 commented Mar 30, 2026 •

edited

Loading

lhy11009 commented Mar 31, 2026 •

edited

Loading

alanjyu Mar 31, 2026 •

edited

Loading