Random initial temperature and compositional plugins #5831

KerrMadeleine · 2024-06-06T20:01:22Z

I have revisited the Random initial composition (which also includes random initial temperature) and attempted to test that the random number generator MT19937 produces the same sequence of random numbers given the same seed. The generator random_number_generator is a private member of the Initial temperature class and seeded with the integer 1. Seeding the RNG with a hash which uses the position value of each point to generate a combined hash has been removed as well as the option to randomly seed the random number generator. Still, with the RNG seed fixed to 1, the plug-in produces different initial temperature fields for the same prm file.

This *.prm file is:
'''

Test to check the status of the random initial temperature perturbation

plugin.

set Output directory = output-test-2
set Dimension = 2

subsection Boundary velocity model
set Tangential velocity boundary indicators = left, right, bottom, top
end

subsection Mesh refinement
set Initial global refinement = 1
end

subsection Postprocess
set List of postprocessors = temperature statistics, visualization
subsection Visualization
set Output format = gnuplot
end
end

subsection Material model
set Model name = simpler
end

set End time = 0
subsection Geometry model
set Model name = box
end

subsection Gravity model
set Model name = vertical
end

subsection Initial temperature model
set List of model names = function, random perturbation
subsection Random perturbation
set Magnitude = 0.2
set Use random seed = false
end

subsection Function
set Function expression = 0.5
end

end

subsection Boundary temperature model
set List of model names = box
set Fixed temperature boundary indicators = top, bottom #MCK added

subsection Box #MCK added
set Bottom temperature = 1
set Top temperature = 0
end

end
'''

gassmoeller · 2024-06-06T20:12:47Z

This is an update of #1813 to modern standards and currently just open for discussion

bangerth · 2024-06-07T01:26:14Z

When you say " the plug-in produces different initial temperature fields for the same prm file", do you mean "different initial temperature fields from run to tun", or different from what?

bangerth · 2024-06-07T01:27:02Z

You've got these warnings and errors:

/home/runner/work/aspect/aspect/include/aspect/initial_composition/random_perturbation.h:90:46: error: expected identifier before numeric constant
   90 |         std::mt19937 random_number_generator(1);
      |                                              ^
/home/runner/work/aspect/aspect/include/aspect/initial_composition/random_perturbation.h:90:46: error: expected ‘,’ or ‘...’ before numeric constant
/home/runner/work/aspect/aspect/include/aspect/initial_composition/random_perturbation.h: In instantiation of ‘class aspect::InitialComposition::RandomPerturbation<2>’:
/home/runner/work/aspect/aspect/source/initial_composition/random_perturbation.cc:121:5:   required from here
/home/runner/work/aspect/aspect/source/initial_composition/random_perturbation.cc:55:5: error: ‘double aspect::InitialComposition::RandomPerturbation<dim>::initial_composition(const dealii::Point<dim>&, unsigned int) const [with int dim = 2]’ can be marked override [-Werror=suggest-override]
   55 |     RandomPerturbation<dim>::
      |     ^~~~~~~~~~~~~~~~~~~~~~~
/home/runner/work/aspect/aspect/source/initial_composition/random_perturbation.cc:100:5: error: ‘void aspect::InitialComposition::RandomPerturbation<dim>::parse_parameters(dealii::ParameterHandler&) [with int dim = 2]’ can be marked override [-Werror=suggest-override]
  100 |     RandomPerturbation<dim>::parse_parameters (ParameterHandler &prm)
      |     ^~~~~~~~~~~~~~~~~~~~~~~
/home/runner/work/aspect/aspect/include/aspect/initial_composition/random_perturbation.h: In instantiation of ‘class aspect::InitialComposition::RandomPerturbation<3>’:
/home/runner/work/aspect/aspect/source/initial_composition/random_perturbation.cc:121:5:   required from here
/home/runner/work/aspect/aspect/source/initial_composition/random_perturbation.cc:55:5: error: ‘double aspect::InitialComposition::RandomPerturbation<dim>::initial_composition(const dealii::Point<dim>&, unsigned int) const [with int dim = 3]’ can be marked override [-Werror=suggest-override]
   55 |     RandomPerturbation<dim>::
      |     ^~~~~~~~~~~~~~~~~~~~~~~
/home/runner/work/aspect/aspect/source/initial_composition/random_perturbation.cc:100:5: error: ‘void aspect::InitialComposition::RandomPerturbation<dim>::parse_parameters(dealii::ParameterHandler&) [with int dim = 3]’ can be marked override [-Werror=suggest-override]
  100 |     RandomPerturbation<dim>::parse_parameters (ParameterHandler &prm)
      |     ^~~~~~~~~~~~~~~~~~~~~~~

gassmoeller

Very close, I just have some stylistic comments. Let me know when you want me to take another look.

gassmoeller · 2024-06-06T21:40:25Z

doc/modules/changes/20170609_gassmoeller

+noise of user prescribed magnitude to the initial temperature field.
+The same type of plugin was added for initial compositions.
+<br>
+(Rene Gassmoeller, 2017/06/19)


Feel free to update the date and add your name here as first author.

gassmoeller · 2024-06-07T12:59:16Z

include/aspect/initial_composition/random_perturbation.h

+         * random composition perturbations. Should return the same
+         * random numbers every time since it is seeded with one.


I would remove the sentence, since the behavior depends on the input parameters.

gassmoeller · 2024-06-07T13:00:17Z

include/aspect/initial_temperature/random_perturbation.h

+        /**
+         * Return the initial temperature as a function of position.
+         */
+


remove this empty line, instead add one before the comment. Also please add documentation for the initialize function above (check the other plugins, you can likely copy the documentation).

also check the initial temperature plugin for the same function

gassmoeller · 2024-06-07T13:00:39Z

source/initial_composition/random_perturbation.cc

+
+#include <aspect/initial_composition/random_perturbation.h>
+
+// #include <boost/functional/hash.hpp>


remove this header file

gassmoeller · 2024-06-07T13:01:26Z

source/initial_composition/random_perturbation.cc

+      else
+        random_number_generator.seed(9088+my_rank);
+    }
+


our typical code style is to have 3 empty lines between function implementations in the source file (and 1 empty line between function declarations in the header file).

gassmoeller · 2024-06-07T13:04:20Z

source/initial_composition/random_perturbation.cc

+    template <int dim>
+    double
+    RandomPerturbation<dim>::
+    initial_composition (const Point<dim> &position,  const unsigned int composition_index) const


The compiler complains because none of the input parameters are used, do this instead:

Suggested change

initial_composition (const Point<dim> &position, const unsigned int composition_index) const

initial_composition (const Point<dim> &/*position*/, const unsigned int /*composition_index*/) const

gassmoeller · 2024-06-07T18:52:03Z

include/aspect/initial_composition/random_perturbation.h

+#include <aspect/initial_composition/interface.h>
+#include <aspect/simulator_access.h>
+
+DEAL_II_DISABLE_EXTRA_DIAGNOSTICS


As far as I know there is no need for DEAL_II_DISABLE_EXTRA_DIAGNOSTICS anymore and also DEAL_II_ENABLE_EXTRA_DIAGNOSTICS. (keep the include).

gassmoeller · 2024-06-07T18:52:18Z

include/aspect/initial_composition/random_perturbation.h

+     */
+    template <int dim>
+    class RandomPerturbation : public Interface<dim>, public aspect::SimulatorAccess<dim>
+


remove this empty line

gassmoeller · 2024-06-07T18:52:34Z

include/aspect/initial_temperature/random_perturbation.h

+#include <aspect/initial_temperature/interface.h>
+#include <aspect/simulator_access.h>
+
+DEAL_II_DISABLE_EXTRA_DIAGNOSTICS


gassmoeller · 2024-06-07T18:55:19Z

source/initial_temperature/random_perturbation.cc

+    template <int dim>
+    double
+    RandomPerturbation<dim>::
+    initial_temperature (const Point<dim> &position) const


as above:

Suggested change

initial_temperature (const Point<dim> &position) const

initial_temperature (const Point<dim> &/*position*/) const

bangerth

This is more of a comment for @gassmoeller: This scheme is not thread-safe. If VectorTools::interpolate() decides to run on multiple processes, we will end up with non-reproducible random numbers. It doesn't, right now, but I thought it's worthwhile putting this into the record in case someone needs to track down a non-reproducibility at some later point.

gassmoeller · 2024-06-09T14:19:26Z

@bangerth You are right of course. However, we never made the last try to produce a thread-safe version work (#1813), because that one produced non-reproducible random number series even on a single process and neither I nor @KerrMadeleine could figure out why. That is why we decided to go with the algorithm that works for the current version of ASPECT and think about thread-safety if it will ever be necessary.

gassmoeller and others added 8 commits June 5, 2024 21:51

Add random noise initial temperature plugin

acf32c5

Add random composition plugin

640a89c

Add option for specific or random seeds

fc17d23

Use position hash as pseudo-random seed

0dfed6c

Update tests

e21480a

Rebase and update tests

1e2ebdc

Add comments for point_hash function

2611b01

Comment out position hash to test RNG seed=1

3dcea95

KerrMadeleine changed the title ~~Random initial temperature : RNG MT19937 does not seed properly~~ Random initial temperature : RNG MT19937 does not seed as expected Jun 6, 2024

gassmoeller changed the title ~~Random initial temperature : RNG MT19937 does not seed as expected~~ [WIP] Random initial temperature : RNG MT19937 does not seed as expected Jun 6, 2024

KerrMadeleine added 3 commits June 6, 2024 21:22

Replace boost functions with std random functions

a30aeed

Replace boost with std for random functions, + cleanup

c4cc10f

Fix comment typo

b280c97

Updated random perturbation to std and to be testable

2a1f7bc

gassmoeller reviewed Jun 7, 2024

View reviewed changes

gassmoeller changed the title ~~[WIP] Random initial temperature : RNG MT19937 does not seed as expected~~ Random initial temperature and compositional plugins Jun 7, 2024

bangerth reviewed Jun 7, 2024

View reviewed changes

gassmoeller mentioned this pull request Jun 9, 2024

Add random initial perturbations #1813

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Random initial temperature and compositional plugins #5831

Random initial temperature and compositional plugins #5831

KerrMadeleine commented Jun 6, 2024

gassmoeller commented Jun 6, 2024

bangerth commented Jun 7, 2024

bangerth commented Jun 7, 2024

gassmoeller left a comment

gassmoeller Jun 6, 2024

gassmoeller Jun 7, 2024

gassmoeller Jun 7, 2024

gassmoeller Jun 7, 2024

gassmoeller Jun 7, 2024

gassmoeller Jun 7, 2024

gassmoeller Jun 7, 2024

gassmoeller Jun 7, 2024

gassmoeller Jun 7, 2024

gassmoeller Jun 7, 2024

gassmoeller Jun 7, 2024

bangerth left a comment

gassmoeller commented Jun 9, 2024

		* random composition perturbations. Should return the same
		* random numbers every time since it is seeded with one.


		#include <aspect/initial_composition/random_perturbation.h>

		// #include <boost/functional/hash.hpp>

	initial_composition (const Point<dim> &position, const unsigned int composition_index) const
	initial_composition (const Point<dim> &/position/, const unsigned int /composition_index/) const

	initial_temperature (const Point<dim> &position) const
	initial_temperature (const Point<dim> &/position/) const

Random initial temperature and compositional plugins #5831

Are you sure you want to change the base?

Random initial temperature and compositional plugins #5831

Conversation

KerrMadeleine commented Jun 6, 2024

Test to check the status of the random initial temperature perturbation

plugin.

gassmoeller commented Jun 6, 2024

bangerth commented Jun 7, 2024

bangerth commented Jun 7, 2024

gassmoeller 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

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

bangerth left a comment

Choose a reason for hiding this comment

gassmoeller commented Jun 9, 2024