Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add 2D and 3D grid positions #48

Merged
merged 6 commits into from
Aug 2, 2023
Merged

Add 2D and 3D grid positions #48

merged 6 commits into from
Aug 2, 2023

Conversation

marjanalbooyeh
Copy link
Contributor

@marjanalbooyeh marjanalbooyeh commented Jul 25, 2023

This PR adds the following functions to the geometry file:

  1. radial_grid_positions: returns a radial grid of 2D positions between two radii. Number of circles between the two radii, number of points in each circle and the coverage angle of each circle can be customized.

  2. spherical_grid_positions: returns a spherical grid of 3D positions between two radii and an angle along z axis. Number of circles between the two radii, number of points in each circle and the coverage angle of each circle can be customized.

These functions can be useful when a fixed set of positions needs to be swipped in a simulation.

@chrisjonesBSU
Copy link
Member

chrisjonesBSU commented Jul 25, 2023

Thanks for adding these :)

A couple things:

  1. It looks like we need to pin the gsd version to 2.9 in the environment file. We can update the gsd stuff in a separate PR.

  2. For consistency, can you follow the same format used in the rest of the cmeutils code in terms limiting lines to 80 characters if possible, and passing funciton args to individual lines once past 80 characters? I think this PEP8 guide covers that Maybe at some point we can add the precommit CI tool that automatically makes these PEP8 related changes to this repo.

  3. Similar to 2, I think we should follow a consistent format in the doc strings where the first line shows the paramter name, data type, and default values, and a second indented line gives the parameter description.

@chrisjonesBSU
Copy link
Member

Looks like the rigid body related tests are failing. We should probably pin hoomd < 4.0.

Per the hoomd 4 release notes:

hoomd.md.constrain.Rigid no longer takes diameters or charges as keys in the body parameters.

@codecov
Copy link

codecov bot commented Jul 25, 2023

Codecov Report

Merging #48 (eb27b30) into master (3486480) will decrease coverage by 3.53%.
Report is 1 commits behind head on master.
The diff coverage is 96.61%.

❗ Current head eb27b30 differs from pull request most recent head 51ecda1. Consider uploading reports for the commit 51ecda1 to get more accurate results

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #48      +/-   ##
==========================================
- Coverage   99.07%   95.54%   -3.53%     
==========================================
  Files           7        7              
  Lines         431      449      +18     
==========================================
+ Hits          427      429       +2     
- Misses          4       20      +16     
Files Changed Coverage Δ
cmeutils/dynamics.py 96.00% <0.00%> (ø)
cmeutils/gsd_utils.py 86.04% <91.66%> (-12.41%) ⬇️
cmeutils/geometry.py 98.27% <100.00%> (+0.77%) ⬆️
cmeutils/plotting.py 100.00% <100.00%> (ø)
cmeutils/sampling.py 100.00% <100.00%> (ø)
cmeutils/structure.py 100.00% <100.00%> (ø)

Copy link
Member

@chrisjonesBSU chrisjonesBSU left a comment

Choose a reason for hiding this comment

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

LGTM! Thanks for adding this @marjanAlbouye

@chrisjonesBSU chrisjonesBSU merged commit b73b93a into master Aug 2, 2023
@chrisjonesBSU chrisjonesBSU deleted the grid_positions branch August 2, 2023 22:02
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