Require Seed #1211

davidsean · 2017-06-27T11:54:47Z

Fixes #1039

Description of changes:

Requires that the user seeds from the script, displays how to do it as part of the runtime error message

PR Checklist

Tests?
- Interface
- Core
Docs?

…seed

RudolfWeeber · 2017-06-27T12:24:05Z

The tutorial scripts and corresponding tex files need to be changed, too.

hmenke · 2017-06-27T21:30:18Z

Great! More global variables.

mkuron · 2017-07-03T14:32:17Z

This is broken on Python 3: https://travis-ci.org/espressomd/espresso/jobs/247486394#L1369

…seed

codecov · 2017-10-18T08:13:38Z

Codecov Report

❗ No coverage uploaded for pull request base (python@3e8ab5d). Click here to learn what that means.
The diff coverage is 38%.

@@           Coverage Diff            @@
##             python   #1211   +/-   ##
========================================
  Coverage          ?     60%           
========================================
  Files             ?     416           
  Lines             ?   28977           
  Branches          ?       0           
========================================
  Hits              ?   17391           
  Misses            ?   11586           
  Partials          ?       0

Impacted Files	Coverage Δ
src/core/random.hpp	`62% <33%> (ø)`
src/core/random.cpp	`39% <50%> (ø)`

Continue to review full report at Codecov.

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

… I seeded them anyways as good measure.

jonaslandsgesell · 2018-02-02T21:53:31Z

This PR seems to have a lot of merge conflicts. Identical seeds may result in synchronization even when the system is setup in different start configurations. This is due to stochastic thermostats like the langevin thermostat acting as an attractor and the Lyapunov exponent(s) therefore having a negative sign such that trajectories exponentially converge. The Lyapunov exponent is system dependent.

This attracting behaviour has for example been shown in 1D Lennard-Jones systems and I can reproduce this behaviour with espresso.

The remaining question is whether this synchronization may be present in systems which are of interest for us. In this case it is important to seed with different seeds when using multiple simulations to estimate the variance of observables. With same seeds (but different start configurations
) the estimated variance of the observables are wrongly too small due to the correlation of different runs with same seeds of the PRNG in espresso.

Synchronization has for example also been reported in a 3d alanine peptide system:
http://pubs.acs.org/doi/abs/10.1021/ct800573m

Although synchronization might not always happen I would be careful here and rather use a random seed. I would say to be on the safe side we should look at the alanine peptide example and see wether something like this can happen in our systems. If we decide it s not possible we can keep the fixed seeds and maybe just close this PR?

kosovan · 2018-02-02T22:16:55Z

If the user is required to provide a seed, what prevents him from providing a random seed?

RudolfWeeber · 2018-07-23T13:32:50Z

github and CI seem to be happy. Any specific thing @KaiSzuttor?

…1211

fweik · 2018-07-23T15:14:59Z

doc/tutorials/01-lennard_jones/scripts/lj_tutorial.py

@@ -65,7 +65,9 @@

 # System setup
 #############################################################
-system              = espressomd.System(box_l=[1.0, 1.0, 1.0])
+system              = espressomd.Systembox_l=([1.0, 1.0, 1.0])


This looks wrong.

KaiSzuttor · 2018-07-24T08:10:30Z

src/core/random.hpp

@@ -83,6 +89,12 @@ void init_random_seed(int seed);

 inline double d_random() {
  using namespace Random;
+
+  if (!user_has_seeded && !unseeded_error_thrown) {
+    unseeded_error_thrown = true;


there is a lot of code duplication here for every function that returns random numbers... also there is no need to throw if temperature is 0.0, e.g. the user only wants the damping part of a langevin thermostat

KaiSzuttor

@davidsean please remove code duplication and only throw if temperature != 0.0

KaiSzuttor · 2018-07-24T13:29:23Z

testsuite/npt.py

@@ -73,7 +74,7 @@ def test_npt(self):
        print(avp,compressibility)

        self.assertAlmostEqual(2.0, avp, delta=0.02)
-        self.assertAlmostEqual(0.2, compressibility, delta=0.01)
+        self.assertAlmostEqual(0.2, compressibility, delta=0.013)


Why did you change the accepted deviation?

RudolfWeeber · 2018-07-24T14:34:32Z

On Tue, Jul 24, 2018 at 06:29:24AM -0700, Kai Szuttor wrote: KaiSzuttor commented on this pull request. > @@ -73,7 +74,7 @@ def test_npt(self): print(avp,compressibility) self.assertAlmostEqual(2.0, avp, delta=0.02) - self.assertAlmostEqual(0.2, compressibility, delta=0.01) + self.assertAlmostEqual(0.2, compressibility, delta=0.013) Why did you change the accepted deviation?

Blew up for a certain number of cores.

This reverts commit dfd232b.

KaiSzuttor · 2018-07-24T14:58:11Z

I broke the drude testcase, no idea how. Thermalized bond and harmonic bond tests pass.

KaiSzuttor · 2018-07-24T15:47:53Z

@fweik , ready for review i think

fweik · 2018-07-24T15:53:59Z

src/core/random.hpp


 namespace Random {
 extern std::mt19937 generator;
 extern std::normal_distribution<double> normal_distribution;
 extern std::uniform_real_distribution<double> uniform_real_distribution;
+extern bool user_has_seeded;
+extern bool unseeded_error_thrown;


This should not be global, I think the best solution is to make it a static local variable in check_user_has_seeded.

hmm, but how to switch the user_has_seeded variable?

I'm only talking about unseeded_error_thrown.

jonaslandsgesell · 2018-07-24T16:10:27Z

What was the exact reason for not requiring a user provided seed when initializing a random number generator (and preventing a random number generator to exist without a user provided seed or state)? That would be the correct dependency inversion right? Then we would not need to check for a seed to be present all the time.

KaiSzuttor · 2018-07-24T17:09:48Z

good idea, but i think that would be some more work. having our manpower in mind, i think this PR serves our current needs. But you can open an issue.

KaiSzuttor · 2018-07-25T11:03:41Z

@fweik

davidsean added 10 commits April 27, 2017 15:43

throwx error is the random number generator is not seeded by the user

4cf5999

comiles but the error creates a crash

ea0467d

abandonded the "throw", and now use runtimeError()

e72b897

runtimeErrorMsg() instead of throw

3f7eaaf

Merge branch 'python' of https://github.com/espressomd/espresso into …

406de2b

…seed

Merge branch 'seed' of github.com:davidsean/espresso into seed

cabbb03

flag unseeded_error_thrown for single error output

bf72377

tests now seed with set_random_state_PRNG()

4fed331

Merge branch 'python' of https://github.com/davidsean/espresso into seed

8202703

Merge branch 'python' of https://github.com/espressomd/espresso into …

e0dde8e

…seed

fweik mentioned this pull request Jul 7, 2017

random: Initialize PRNG with random number from system. #1039

Closed

1 task

davidsean added 4 commits October 5, 2017 12:29

Merge branch 'seed' of github.com:davidsean/espresso into seed

1167db9

tests should be deterministic

2dd67fb

tutorial 1 and 2 are now deterministic

8c33aaf

Merge branch 'python' of https://github.com/espressomd/espresso into …

b9f192c

…seed

davidsean added 9 commits November 28, 2017 11:52

Merge branch 'python' of github.com:davidsean/espresso into seed

ab9a80d

seed comfixed test

1980d9d

seed catalytic reaction

7d564de

seed npt

87b6734

system typo

851a657

same

d1c5e01

more explicit seeding

7c6fcec

these do not cause a crash, probably since they do not integrate. But…

d727056

… I seeded them anyways as good measure.

Merge branch 'python' into seed

866f030

RudolfWeeber added 2 commits July 23, 2018 15:56

Merge branch 'python' of git://github.com/espressomd/espresso into pr…

b448235

…1211

Removed outdated shape based constraint tests

27c1e36

fweik suggested changes Jul 23, 2018

View reviewed changes

KaiSzuttor added 2 commits July 24, 2018 09:41

testsuite: removed unused files.

5f32992

tutorials: corrected syntax.

10dc015

KaiSzuttor reviewed Jul 24, 2018

View reviewed changes

KaiSzuttor suggested changes Jul 24, 2018

View reviewed changes

KaiSzuttor and others added 5 commits July 24, 2018 10:44

testsuite/tutorials: removed seeding where not necessary.

9adfad5

testsuite: removed seeding where not necessary - episode2.

bde64ac

removed code duplication

5f16263

random: guard calls to rngs.

f103d3d

testsuite: minor improvements.

d53ca47

KaiSzuttor reviewed Jul 24, 2018

View reviewed changes

testsuite: use previous accuracy requirement for npt test.

dfd232b

Revert "testsuite: use previous accuracy requirement for npt test."

dc2c082

This reverts commit dfd232b.

testsuite: slightly increase number of integrations for drude test.

57ea9a6

KaiSzuttor approved these changes Jul 24, 2018

View reviewed changes

fweik suggested changes Jul 24, 2018

View reviewed changes

random: use static function member instead of global.

6b56534

fweik and others added 2 commits July 25, 2018 16:49

Merge branch 'python' into seed

064f5ac

testsuite: Fixed merge regression.

a62c7b1

fweik merged commit 99fef59 into espressomd:python Jul 25, 2018

mkuron mentioned this pull request Nov 2, 2018

LB GPU uses random seed #2356

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Require Seed #1211

Require Seed #1211

davidsean commented Jun 27, 2017

RudolfWeeber commented Jun 27, 2017 via email

hmenke commented Jun 27, 2017

mkuron commented Jul 3, 2017

codecov bot commented Oct 18, 2017 •

edited

jonaslandsgesell commented Feb 2, 2018 •

edited

kosovan commented Feb 2, 2018

RudolfWeeber commented Jul 23, 2018

fweik Jul 23, 2018

KaiSzuttor Jul 24, 2018

KaiSzuttor left a comment

KaiSzuttor Jul 24, 2018

RudolfWeeber commented Jul 24, 2018 via email

KaiSzuttor commented Jul 24, 2018

KaiSzuttor commented Jul 24, 2018

fweik Jul 24, 2018

KaiSzuttor Jul 24, 2018

fweik Jul 24, 2018

jonaslandsgesell commented Jul 24, 2018

KaiSzuttor commented Jul 24, 2018

KaiSzuttor commented Jul 25, 2018

Require Seed #1211

Require Seed #1211

Conversation

davidsean commented Jun 27, 2017

Description of changes:

PR Checklist

RudolfWeeber commented Jun 27, 2017 via email

hmenke commented Jun 27, 2017

mkuron commented Jul 3, 2017

codecov bot commented Oct 18, 2017 • edited

Codecov Report

jonaslandsgesell commented Feb 2, 2018 • edited

kosovan commented Feb 2, 2018

RudolfWeeber commented Jul 23, 2018

fweik Jul 23, 2018

Choose a reason for hiding this comment

KaiSzuttor Jul 24, 2018

Choose a reason for hiding this comment

KaiSzuttor left a comment

Choose a reason for hiding this comment

KaiSzuttor Jul 24, 2018

Choose a reason for hiding this comment

RudolfWeeber commented Jul 24, 2018 via email

KaiSzuttor commented Jul 24, 2018

KaiSzuttor commented Jul 24, 2018

fweik Jul 24, 2018

Choose a reason for hiding this comment

KaiSzuttor Jul 24, 2018

Choose a reason for hiding this comment

fweik Jul 24, 2018

Choose a reason for hiding this comment

jonaslandsgesell commented Jul 24, 2018

KaiSzuttor commented Jul 24, 2018

KaiSzuttor commented Jul 25, 2018

codecov bot commented Oct 18, 2017 •

edited

jonaslandsgesell commented Feb 2, 2018 •

edited