Extend thermostat and rotation test #1510
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Up to values of 1E-3, approximately.
finalize() and get_datata() This may well have been the Heisenbug causing the segfaults
…tInterface-derived classes
Remove correlators from system.auto_update_correlators after finalize() call
energy and momenum conservation for non-isotropi itensor
By not calling finalize()
Codecov Report
@@ Coverage Diff @@
## python #1510 +/- ##
==========================================
+ Coverage 40.59% 42.38% +1.78%
==========================================
Files 364 371 +7
Lines 30509 30011 -498
Branches 5584 5806 +222
==========================================
+ Hits 12386 12719 +333
+ Misses 18097 17268 -829
+ Partials 26 24 -2
Continue to review full report at Codecov.
|
hmenke
suggested changes
Oct 11, 2017
| @@ -4,7 +4,7 @@ file(GLOB EspressoCore_SRC | |||
|
|
|||
| if( WITH_COVERAGE ) | |||
| set(CMAKE_BUILD_TYPE "Debug" CACHE STRING "Choose the type of build, options are: Debug Release RelWithDebInfo MinSizeRel." FORCE) | |||
| set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -g -O0 --coverage -fprofile-arcs -ftest-coverage") | |||
| set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -g -O1 --coverage -fprofile-arcs -ftest-coverage") | |||
There was a problem hiding this comment.
That is not a good idea. -O1 includes for example -finline-functions-called-once which will mess up coverage.
There was a problem hiding this comment.
From the numbers that does not seem to be a big issue.
fweik
approved these changes
Oct 11, 2017
|
> @@ -4,7 +4,7 @@ file(GLOB EspressoCore_SRC
if( WITH_COVERAGE )
set(CMAKE_BUILD_TYPE "Debug" CACHE STRING "Choose the type of build, options are: Debug Release RelWithDebInfo MinSizeRel." FORCE)
- set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -g -O0 --coverage -fprofile-arcs -ftest-coverage")
+ set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -g -O1 --coverage -fprofile-arcs -ftest-coverage")
That is not a good idea. `-O1` includes for example `-finline-functions-called-once` which will mess up coverage.
It was simply necessary to get the Green Kubo integrals to converge
in less than 10 minutes on Travis. I think, we can trade good
testing for all aspects of the thermostat (and apparently the correlator) against a bit of accuracy
in coverage.
|
Closed
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Verify diffusion coeff via Green Kubo per component for rotation and translation.
All combinations of ROTATION, MASS, RINERTIA, LANGEVIN_PER_PARTICLE and PARTICLE_ANISOTROPY should be handled.
Extended test for rotational inertia: Test momenum and energy conservation for non-isotropic inertia tensor
Further changes:
Correlator: Prevent double finalize() as well as get_data() after finalize().
py thermostat interface: Replace isinstance(... List) by hasattr(.., @iter_@) so it works with tuplesp and np arrays.