Add random_uniform_distribution_deflected to all features and enable random number seed as input #584

Wang-yijun · 2024-02-14T21:27:15Z

This PR adds random_uniform_distribution_deflected texture to all features (continental plate, oceanic plate, fault, subducting plate and mantle layer), sets default global random number seed as NaN unless entered in the input file, and lets random_uniform_distribution_deflected texture take random number seeds as inputs to generate different random textures with different deflection from the center.

MFraters

Looks good. Here are some initial comments we also discussed.

MFraters · 2024-02-15T03:12:28Z

include/world_builder/world.h

+      /**
+       * Todo
+       */
+      int local_seed;


I don't think this is needed. Can you remove this and just locally declare it as an int?

Fixed this.

MFraters · 2024-02-15T03:15:14Z

...ld_builder/features/continental_plate_models/grains/random_uniform_distribution_deflected.cc

+        RandomUniformDistributionDeflected::declare_entries(Parameters &prm, const std::string & /*unused*/)
+        {
+          // Document plugin and require entries if needed.
+          // Add compositions the required parameters.


Suggested change

// Add compositions the required parameters.

// Add compositions to the required parameters.

Fixed this and some other descriptions of the feature in random_uniform_distribution_deflected

MFraters · 2024-02-15T03:36:28Z

...ld_builder/features/continental_plate_models/grains/random_uniform_distribution_deflected.cc

+          normalize_grain_sizes = prm.get_vector<bool>("normalize grain sizes");
+          deflections = prm.get_vector<double>("deflections");
+          seed = prm.get<unsigned int>("random number seed");
+          world->get_random_number_engine().seed(seed);


As discussed, use global one

Deleted this and other use of local seed inside features

MFraters

The code looks good to me. Can you add tests to that all code is tested?

MFraters · 2024-02-15T16:30:31Z

include/world_builder/world.h

@@ -276,6 +276,7 @@ namespace WorldBuilder
      std::string interpolation;


+


could you remove this?

Test added and empty line removed.

tjhei · 2024-02-15T22:33:23Z

include/world_builder/features/fault_models/grains/random_uniform_distribution_deflected.h

+      namespace Grains
+      {
+        /**
+         * This class represents a fault and can implement


Is this comment block copied from somewhere?

Yes, I this is copied from random_uniform_distribution.h in the same folder as a template and I made changes based on that.

tjhei · 2024-02-15T22:34:10Z

...de/world_builder/features/mantle_layer_models/grains/random_uniform_distribution_deflected.h

+                       const double feature_max_depth) const override final;
+
+          private:
+            // random uniform distribution deflected grains submodule parameters


I would delete this comment as it doesn't add any useful information.

github-actions · 2024-02-16T16:37:36Z

Benchmark	Main	Feature	Difference (99.9% CI)
Slab interpolation simple none	1.008 ± 0.003 (s=476)	1.018 ± 0.004 (s=415)	+0.9% .. +1.1%
Slab interpolation curved simple none	1.014 ± 0.004 (s=443)	1.023 ± 0.005 (s=443)	+0.8% .. +1.0%
Spherical slab interpolation simple none	1.162 ± 0.008 (s=382)	1.166 ± 0.007 (s=394)	+0.2% .. +0.5%
Slab interpolation simple curved CMS	1.057 ± 0.004 (s=428)	1.061 ± 0.004 (s=424)	+0.2% .. +0.4%
Spherical slab interpolation simple CMS	1.544 ± 0.009 (s=290)	1.549 ± 0.011 (s=294)	+0.1% .. +0.5%
Spherical fault interpolation simple none	1.171 ± 0.008 (s=406)	1.176 ± 0.009 (s=364)	+0.2% .. +0.5%
Cartesian min max surface	2.295 ± 0.017 (s=193)	2.301 ± 0.016 (s=201)	+0.0% .. +0.5%
Spherical min max surface	7.198 ± 0.022 (s=63)	7.211 ± 0.024 (s=64)	-0.0% .. +0.4%

codecov · 2024-02-16T23:36:14Z

Codecov Report

Attention: 15 lines in your changes are missing coverage. Please review.

Comparison is base (31e7e1e) 92.83% compared to head (e30af2a) 92.99%.
Report is 6 commits behind head on main.

❗ Current head e30af2a differs from pull request most recent head 9f41335. Consider uploading reports for the commit 9f41335 to get more accurate results

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #584      +/-   ##
==========================================
+ Coverage   92.83%   92.99%   +0.16%     
==========================================
  Files          92       98       +6     
  Lines        6474     6856     +382     
==========================================
+ Hits         6010     6376     +366     
- Misses        464      480      +16

Files	Coverage Δ
source/world_builder/parameters.cc	`97.94% <100.00%> (+0.02%)`	⬆️
source/world_builder/world.cc	`100.00% <100.00%> (ø)`
...ls/grains/random_uniform_distribution_deflected.cc	`97.22% <97.22%> (ø)`
...ls/grains/random_uniform_distribution_deflected.cc	`97.10% <97.10%> (ø)`
...ls/grains/random_uniform_distribution_deflected.cc	`97.22% <97.22%> (ø)`
...ls/grains/random_uniform_distribution_deflected.cc	`97.22% <97.22%> (ø)`
...ls/grains/random_uniform_distribution_deflected.cc	`97.10% <97.10%> (ø)`
source/world_builder/types/int.cc	`70.58% <70.58%> (ø)`

... and 4 files with indirect coverage changes

Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 31e7e1e...9f41335. Read the comment docs.

MFraters

I would propose merging this as is. Please make a follow up pull request with

a more extensive explanation of the random number seed declare entry
A test which is exactly the same, but has a different random number.

Wang-yijun · 2024-02-17T02:24:55Z

Example texture generated using random uniform distribution deflected (visualized using MTEX package in Matlab):

deflection = 0.8, random number seed = 1

deflection = 0.6, random number seed = 1

deflection = 0.4, random number seed = 1

deflection = 0.2, random number seed = 1

deflection = 0.6, random number seed = 10

Wang-yijun changed the title ~~Add random_uniform_distribution_deflected to all features and enable random number seed as input~~ [WIP] Add random_uniform_distribution_deflected to all features and enable random number seed as input Feb 14, 2024

Wang-yijun changed the title ~~[WIP] Add random_uniform_distribution_deflected to all features and enable random number seed as input~~ Add random_uniform_distribution_deflected to all features and enable random number seed as input Feb 15, 2024

MFraters reviewed Feb 15, 2024

View reviewed changes

tjhei reviewed Feb 15, 2024

View reviewed changes

MFraters mentioned this pull request Feb 16, 2024

Random number seed as input for generating different random textures #575

Closed

Wang-yijun force-pushed the random_number_seed branch from 190e9fe to f78e259 Compare February 16, 2024 20:57

Wang-yijun added 7 commits February 16, 2024 18:12

Fix merge issue and test with tag

ace099f

Move random number seed to initalization

10799d3

Fix indentation

3f98a39

Fix local seed type and add get function for int

a5f93db

Add argument for ValueAtPoints

7976617

Move random number seed to initalization

8db24ad

Fix indentation

9f41335

Wang-yijun force-pushed the random_number_seed branch from e30af2a to 9f41335 Compare February 17, 2024 00:14

MFraters approved these changes Feb 17, 2024

View reviewed changes

MFraters added the ready to merge Pull request is ready to merge. May be waiting for tests to complete or other reviews. label Feb 17, 2024

gassmoeller merged commit 2915c68 into GeodynamicWorldBuilder:main Feb 17, 2024
30 of 32 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add random_uniform_distribution_deflected to all features and enable random number seed as input #584

Add random_uniform_distribution_deflected to all features and enable random number seed as input #584

Wang-yijun commented Feb 14, 2024

MFraters left a comment

MFraters Feb 15, 2024

Wang-yijun Feb 15, 2024

MFraters Feb 15, 2024

Wang-yijun Feb 15, 2024

MFraters Feb 15, 2024

Wang-yijun Feb 15, 2024

MFraters left a comment

MFraters Feb 15, 2024

Wang-yijun Feb 15, 2024

tjhei Feb 15, 2024

Wang-yijun Feb 16, 2024

tjhei Feb 15, 2024

Wang-yijun Feb 16, 2024

github-actions bot commented Feb 16, 2024 •

edited

Loading

codecov bot commented Feb 16, 2024 •

edited

Loading

MFraters left a comment •

edited

Loading

Wang-yijun commented Feb 17, 2024

	// Add compositions the required parameters.
	// Add compositions to the required parameters.

		@@ -276,6 +276,7 @@ namespace WorldBuilder
		std::string interpolation;

Add random_uniform_distribution_deflected to all features and enable random number seed as input #584

Add random_uniform_distribution_deflected to all features and enable random number seed as input #584

Conversation

Wang-yijun commented Feb 14, 2024

MFraters left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

MFraters left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

github-actions bot commented Feb 16, 2024 • edited Loading

codecov bot commented Feb 16, 2024 • edited Loading

Codecov Report

MFraters left a comment • edited Loading

Choose a reason for hiding this comment

Wang-yijun commented Feb 17, 2024

github-actions bot commented Feb 16, 2024 •

edited

Loading

codecov bot commented Feb 16, 2024 •

edited

Loading

MFraters left a comment •

edited

Loading