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

Tutorials #38

Merged
merged 49 commits into from
Sep 29, 2023
Merged

Tutorials #38

merged 49 commits into from
Sep 29, 2023

Conversation

marjanalbooyeh
Copy link
Collaborator

@marjanalbooyeh marjanalbooyeh commented Sep 5, 2023

This PR initiates some educational tutorials in form of Jupyter notebooks to cover functionalities of the package.

Currently, the theme I have in mind is like this:

  • A basic tutorial: quick intro to build molecules, pack the system and run the sim
  • An advanced tutorial: intro to features and functionalities of the package, highlighting some of the customized features of the package.
  • An easy example of coarse graining (in progress)
  • Maybe some fancier examples of polymer builder?
  • Welding tutorial
  • Tensile test tutorial
  • Maybe the LJ ML force (depends on if I can get the model to work with the data)

We can definitely change this format, update names and add more stuff. This is just the initial draft.

@review-notebook-app
Copy link

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.


Powered by ReviewNB

@marjanalbooyeh marjanalbooyeh linked an issue Sep 5, 2023 that may be closed by this pull request
@marjanalbooyeh marjanalbooyeh linked an issue Sep 5, 2023 that may be closed by this pull request
@chrisjonesBSU
Copy link
Member

I'm running through the notebook and things look good!

For the cells that create a Lattice system, I think we should use a polymer instead of benzene molecules. It makes it easier to visualize the crystalline system. We should probably use the PPS class in that part with a length of 5-10ish.

@marjanalbooyeh marjanalbooyeh marked this pull request as ready for review September 7, 2023 21:25
@codecov
Copy link

codecov bot commented Sep 8, 2023

Codecov Report

Merging #38 (eac5fa9) into main (f470832) will not change coverage.
The diff coverage is n/a.

Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff           @@
##             main      #38   +/-   ##
=======================================
  Coverage   93.70%   93.70%           
=======================================
  Files          19       19           
  Lines        1397     1397           
=======================================
  Hits         1309     1309           
  Misses         88       88           

@chrisjonesBSU
Copy link
Member

Every time we call system.hoomd_forcefield or system.hoomd_snapshot both the forces and snapshot have to be created, which can be kind of slow. So, for the case of tutorials, we should set these equal to a variable the first time they are called, then use that variable anywhere else we need it. Or, we can change the behavior in system.py so that once one of these have been created, it doesn't go through the process again.

For example, in the 1-Hoomd-Organics-basics tutorial we have these lines:

system.hoomd_forcefield
lj_force = system.hoomd_forcefield[3]
sim = Simulation(initial_state=system.hoomd_snapshot, forcefield=system.hoomd_forcefield, gsd_write_freq=100, log_write_freq=100)

So, system._create_hoomd_forcefield is running 3 separate times.

We could do something like:

hoomd_forces = system.hoomd_forcefield
lj_force = hoomd_forces[3]
sim = Simulation(initial_state=system.hoomd_snapshot, forcefield=hoomd_forces, gsd_write_freq=100, log_write_freq=100)

The same applies to system.hoomd_snapshot which right now, is being called 2 different times.

@chrisjonesBSU
Copy link
Member

chrisjonesBSU commented Sep 8, 2023

A couple other things:

  1. The way opls does charges isn't great, so we easily end up with not charge neutral systems (hoomd was throwing some warnings I believe), so we should set scale_charges = True in the line that creates the system

  2. Since we are using auto_scale = True We need to account for that when setting the target box lengths for shrink.

Change:

sim.run_update_volume(n_steps=1000, period=1, kT=1, tau_kt=1, final_box_lengths=system.target_box)

to

sim.run_update_volume(
    n_steps=1000,
    period=1,
    kT=1,
    tau_kt=1,
    final_box_lengths=system.target_box/system.reference_length
)

Maybe we should make it so that when auto_scale = True in the System class, it is automatically taken into account with target_box?

After making these 2 changes, I can run through the simulation cells without getting particle out of box errors.

Also, we could swap the shrink and NVT cells. Running shrink first, then NVT once we reached the target density.

@erjank
Copy link
Contributor

erjank commented Sep 18, 2023

Also, benzene is c1ccccc1, not C1CCCCC1

@marjanalbooyeh marjanalbooyeh merged commit 65e833a into cmelab:main Sep 29, 2023
4 checks passed
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.

Tutorial initializes cyclohexanes, but calls them benzenes Make more tutorials
4 participants