Skip to content

Cleanup functions related to rotational velocity, improve sample input files#162

Merged
danielhollas merged 4 commits intomasterfrom
rotation-removal-cleanup
Jun 21, 2023
Merged

Cleanup functions related to rotational velocity, improve sample input files#162
danielhollas merged 4 commits intomasterfrom
rotation-removal-cleanup

Conversation

@danielhollas
Copy link
Copy Markdown
Contributor

Cleaning up the remove_rotations functions, that we call at the beginning of the simulations to remove rotational velocity.
Confusingly, we've also used the same function to print angular momentum along the MD simulations.
In this PR I decoupled the code to smaller functions to make this all less confusing.

Also tried to cleanup the sample input files to make them (hopefully) a little more accessible for first time users.
But MUCH more needs to be done in this area.

@danielhollas danielhollas force-pushed the rotation-removal-cleanup branch from af5106e to 046f163 Compare May 23, 2023 20:52
@danielhollas danielhollas requested a review from JanosJiri May 23, 2023 20:54
@danielhollas
Copy link
Copy Markdown
Contributor Author

danielhollas commented May 23, 2023

@JanosJiri can you take a look? Thanks! Also, if there is anybody else that would perhaps be interesting in reviewing the code, let me know! (especially for simple refactoring like this, it might be nice for somebody to get familiar with the codebase).

@codecov
Copy link
Copy Markdown

codecov Bot commented May 23, 2023

Codecov Report

Merging #162 (26601db) into master (5bee87b) will increase coverage by 0.01%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #162      +/-   ##
==========================================
+ Coverage   92.41%   92.42%   +0.01%     
==========================================
  Files          43       43              
  Lines        6053     6063      +10     
  Branches      735      734       -1     
==========================================
+ Hits         5594     5604      +10     
  Misses        447      447              
  Partials       12       12              
Flag Coverage Δ
unittests 24.78% <0.00%> (-0.05%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
src/estimators.F90 95.68% <ø> (ø)
src/analysis.F90 98.85% <100.00%> (+0.03%) ⬆️
src/init.F90 90.50% <100.00%> (ø)
src/vinit.F90 97.90% <100.00%> (+0.07%) ⬆️

Copy link
Copy Markdown
Contributor

@JanosJiri JanosJiri left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I left just small suggestions/comments. No need to incorporate them.

Comment thread sample_inputs/input.in.cmd
&nhcopt
temp=298.15, ! temperature [K] for Maxwell-Boltzmann sampling and thermostat
inose=1, ! Thermostating: Nose-Hoover 1, microcanonical 0,GLE 2
nchain=4, ! number of nose-hoover chains
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a comment: there are lots of deleted options. It might t be useful to have some brief manual for them. :)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I've deleted a bunch of options that the user should not generally touch, since these sample inputs are generally meant for first time users.

Note that ABIN's manual is currently available on Overleaf, but I haven't updated it in a long while. Some of these options are documented there though. Perhaps that is something we could work together when you are in Bristol. :-)

https://www.overleaf.com/read/xbsxbyvdnfzh

Comment thread sample_inputs/input.in.pimd Outdated
@JanosJiri
Copy link
Copy Markdown
Contributor

@JanosJiri can you take a look? Thanks! Also, if there is anybody else that would perhaps be interesting in reviewing the code, let me know! (especially for simple refactoring like this, it might be nice for somebody to get familiar with the codebase).

Currently, there is no one working on ABIN modifications but I will ask.

@danielhollas danielhollas merged commit f59e263 into master Jun 21, 2023
@danielhollas danielhollas deleted the rotation-removal-cleanup branch June 21, 2023 12:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants