Add ability to use number density when calculating target box lenghts for shrinking and NVT. #112

chrisjonesBSU · 2024-01-11T17:34:16Z

Being able to easily and reliability get volumes that correspond to a desired density is very helpful, we currently do this for mass density, but that isn't very intuitive or helpful when running non-atomistic coarse-grain simulations such as the LJChain, 'KremerGrestBeadSpring' and ellipsoids (see #102).

Right now, the changes are in utils.calculate_box_length.

This PR also solves #64.

Still left to do:

Unit tests
Decide how to handle a Simulation property of N_beads
Decide if we need to change how Simulation.run_update_volume handles its density parameter.

codecov · 2024-01-11T21:41:45Z

Codecov Report

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

Comparison is base (df50ced) 94.23% compared to head (a919fed) 94.28%.
Report is 4 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #112      +/-   ##
==========================================
+ Coverage   94.23%   94.28%   +0.04%     
==========================================
  Files          23       25       +2     
  Lines        1770     1801      +31     
==========================================
+ Hits         1668     1698      +30     
- Misses        102      103       +1

Files	Coverage Δ
flowermd/base/molecule.py	`96.77% <100.00%> (ø)`
flowermd/base/simulation.py	`91.48% <100.00%> (+0.06%)`	⬆️
flowermd/base/system.py	`92.00% <100.00%> (+0.01%)`	⬆️
flowermd/internal/__init__.py	`100.00% <100.00%> (ø)`
flowermd/internal/exceptions.py	`89.74% <ø> (ø)`
flowermd/internal/ff_utils.py	`89.58% <ø> (ø)`
flowermd/internal/utils.py	`100.00% <100.00%> (ø)`
flowermd/library/surfaces.py	`100.00% <ø> (ø)`
...lowermd/modules/surface_wetting/surface_wetting.py	`94.73% <100.00%> (+0.61%)`	⬆️
flowermd/utils/__init__.py	`100.00% <ø> (ø)`
... and 2 more

chrisjonesBSU · 2024-01-11T22:43:16Z

Ok, for now I took out the N_beads property from Simulation. We can make an issue and discuss how we want to handle that another time.

We can still figure out what to do in run_update_volume now that there are 2 types of density.

Keep current behavior and have a final_density parameter that just assumes they are using mass density.
Remove final_density and make it so that the utils funcs for calculating boxes that match a given density are more forward facing. People can just use it to get the final_box_lengths parameter they want to use in run_update_volume.

flowermd/utils/utils.py

flowermd/base/system.py

marjanalbooyeh · 2024-01-12T20:01:16Z

We can still figure out what to do in run_update_volume now that there are 2 types of density.

For the purpose of keeping function interfaces simple, I think option 2 is better.

chrisjonesBSU · 2024-01-17T18:02:12Z

Ok, I think everything is implemented correctly. I just need to update doc strings and run through the tutorials to make sure they are working correctly.

chrisjonesBSU · 2024-01-17T20:48:13Z

flowermd/base/simulation.py

-            ).value
-            final_box = hoomd.Box(Lx=L, Ly=L, Lz=L)
-
+            # TODO: Do we need to do anything here?


Forgot about this TODO that is remaining.

I can't think of anything to do for cases where final_box_lengths does not have units. I guess we can remove this else for now?

chrisjonesBSU · 2024-01-17T20:51:38Z

flowermd/modules/surface_wetting/surface_wetting.py

@@ -95,21 +95,27 @@ def run_droplet(

        """
        # Shrink down to high density
+        target_box = get_target_box_mass_density(
+            density=shrink_density * (u.g / (u.cm**3)), mass=self.mass.to("g")


Maybe we need an if statement here. Maybe someone passes in other density units for final_density and shrink_density and we don't always want to use g/cm^3

I guess the behavior here should basically be the same as what we do in Pack now.

Agree. same as in Pack we could first check if the units exists and if not assume the default is g/cm^3

flowermd/tests/base/test_system.py

review-notebook-app · 2024-01-18T17:16:42Z

Check out this pull request on

See visual diffs & provide feedback on Jupyter Notebooks.

Powered by ReviewNB

marjanalbooyeh

LGTM overall! I added a few comments/questions.

Thank you Chris

flowermd/base/simulation.py

marjanalbooyeh · 2024-01-18T22:01:51Z

flowermd/base/simulation.py

-            ).value
-            final_box = hoomd.Box(Lx=L, Ly=L, Lz=L)
-
+            # TODO: Do we need to do anything here?


I can't think of anything to do for cases where final_box_lengths does not have units. I guess we can remove this else for now?

flowermd/base/system.py

marjanalbooyeh · 2024-01-18T22:09:11Z

flowermd/modules/surface_wetting/surface_wetting.py

@@ -95,21 +95,27 @@ def run_droplet(

        """
        # Shrink down to high density
+        target_box = get_target_box_mass_density(
+            density=shrink_density * (u.g / (u.cm**3)), mass=self.mass.to("g")


Agree. same as in Pack we could first check if the units exists and if not assume the default is g/cm^3

flowermd/base/simulation.py

flowermd/tests/base/test_simulation.py

…to number-density

marjanalbooyeh

LGTM. Thank you

chrisjonesBSU added 4 commits January 10, 2024 12:07

add support for number density

9fd764f

clean up a few lines

fe53c61

fix n density variable

b51c873

Merge branch 'main' of github.com:cmelab/flowerMD into number-density

9f5d382

chrisjonesBSU added the enhancement New feature or request label Jan 11, 2024

chrisjonesBSU requested a review from marjanalbooyeh January 11, 2024 17:34

chrisjonesBSU added 6 commits January 11, 2024 12:09

fix f strings, start adding unit tests

9d68d38

fix unit tests

366758b

fix param names to match new ones

89feaa2

fix param names

5946794

remove set target_box from Lattice system, fix tests

dc80e48

Fix tests

410b5e3

remove N_beads property for now

2d8801f

marjanalbooyeh reviewed Jan 12, 2024

View reviewed changes

flowermd/utils/utils.py Outdated Show resolved Hide resolved

marjanalbooyeh reviewed Jan 12, 2024

View reviewed changes

flowermd/base/system.py Outdated Show resolved Hide resolved

chrisjonesBSU added 9 commits January 12, 2024 14:19

implement new design choices for getting target boxes

084a8b1

remove target box attr from System, fix unit test

c61d2b2

update tests

a934902

make sure target box array has units, convert to nm in Pack

790c06a

add warning message if units not given for density

762626b

fix tests, convert box in run update vol

3132e0d

fix density in droplet module

fa0dd65

fix test

7de8652

fix tests

e11e6b5

chrisjonesBSU linked an issue Jan 17, 2024 that may be closed by this pull request

Move the density parameter out of System #64

Closed

chrisjonesBSU commented Jan 17, 2024

View reviewed changes

flowermd/tests/base/test_system.py Outdated Show resolved Hide resolved

chrisjonesBSU commented Jan 17, 2024

View reviewed changes

flowermd/tests/base/test_system.py Outdated Show resolved Hide resolved

chrisjonesBSU added 2 commits January 17, 2024 14:09

add units check, remove debug line

340f0c7

update basics tutorial

97e406d

chrisjonesBSU added 3 commits January 18, 2024 10:17

use amu units in System.mass property

9fb2744

finish checking the rest of the tutorials

2d712e8

fix unit tests

4344d84

marjanalbooyeh approved these changes Jan 18, 2024

View reviewed changes

chrisjonesBSU and others added 10 commits January 18, 2024 17:22

add doc strings for target box funcs

732670b

add unit test for no refs, add example to doc strings

15e0581

create internal module, add doc for utils

6146a4d

delete swp files, fix imports in tests

3e8d8fd

remove swp file

893a4ed

add swp files to git ignore

2ec25b3

fix utils rst file

d2cb45e

add docstrings

746b85f

finish default density behavior in surface wetting

f76a5c0

Merge branch 'number-density' of github.com:chrisjonesBSU/flowerMD in…

a919fed

…to number-density

marjanalbooyeh approved these changes Jan 22, 2024

View reviewed changes

chrisjonesBSU merged commit c06e8c3 into cmelab:main Jan 22, 2024
4 checks passed

chrisjonesBSU deleted the number-density branch January 22, 2024 21:23

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add ability to use number density when calculating target box lenghts for shrinking and NVT. #112

Add ability to use number density when calculating target box lenghts for shrinking and NVT. #112

chrisjonesBSU commented Jan 11, 2024 •

edited

Loading

codecov bot commented Jan 11, 2024 •

edited

Loading

chrisjonesBSU commented Jan 11, 2024

marjanalbooyeh commented Jan 12, 2024

chrisjonesBSU commented Jan 17, 2024

chrisjonesBSU Jan 17, 2024

marjanalbooyeh Jan 18, 2024

chrisjonesBSU Jan 17, 2024

chrisjonesBSU Jan 17, 2024

marjanalbooyeh Jan 18, 2024

review-notebook-app bot commented Jan 18, 2024

marjanalbooyeh left a comment

marjanalbooyeh Jan 18, 2024

marjanalbooyeh Jan 18, 2024

marjanalbooyeh left a comment

Add ability to use number density when calculating target box lenghts for shrinking and NVT. #112

Add ability to use number density when calculating target box lenghts for shrinking and NVT. #112

Conversation

chrisjonesBSU commented Jan 11, 2024 • edited Loading

codecov bot commented Jan 11, 2024 • edited Loading

Codecov Report

chrisjonesBSU commented Jan 11, 2024

marjanalbooyeh commented Jan 12, 2024

chrisjonesBSU commented Jan 17, 2024

chrisjonesBSU Jan 17, 2024

Choose a reason for hiding this comment

marjanalbooyeh Jan 18, 2024

Choose a reason for hiding this comment

chrisjonesBSU Jan 17, 2024

Choose a reason for hiding this comment

chrisjonesBSU Jan 17, 2024

Choose a reason for hiding this comment

marjanalbooyeh Jan 18, 2024

Choose a reason for hiding this comment

review-notebook-app bot commented Jan 18, 2024

marjanalbooyeh left a comment

Choose a reason for hiding this comment

marjanalbooyeh Jan 18, 2024

Choose a reason for hiding this comment

marjanalbooyeh Jan 18, 2024

Choose a reason for hiding this comment

marjanalbooyeh left a comment

Choose a reason for hiding this comment

chrisjonesBSU commented Jan 11, 2024 •

edited

Loading

codecov bot commented Jan 11, 2024 •

edited

Loading