[REVIEW]: pylj: A teaching tool for classical atomistic simulation about jose-reviews HOT 64 CLOSED

Yay! Thanks @labarba for your awesome editorial work, and thanks again to @shivupa and @TJFord!

from jose-reviews.

Isn't our bot whedon just the cutest thing?

from jose-reviews.

Congrats! I'm happy to have been a part of this.

from jose-reviews.

Can we possibly agree on "classical atomistic simulation"? For the following reasons:

• Since we are only really dealing with individual atoms of argon in pylj it is a bit false to say molecular.
• I would like to keep "classical" in there as pylj only performs classical potential based simulation, e.g. there are not quantum mechanical methods (e.g. DTF) to be seen.

from jose-reviews.

--> Check article proof 📄 <--

from jose-reviews.

@labarba I think the author @arm61 has addressed all my concerns. I suggest that it is ready for publication.

from jose-reviews.

@labarba I also agree @arm61 has addressed all concerns and it is ready for publication.

from jose-reviews.

also a massive thanks to @shivupa and and @TJFord for all their hard work on making pylj a MUCH more useful piece of software than previously. Great peer reviewing folks 😄

from jose-reviews.

Please stay tuned. I am at a conference now. I have to go over the comments/improvements and paper myself before accepting the submission.

from jose-reviews.

I need to do a little local processing to publish your paper, but will have to do that tomorrow, due to commitments now.

from jose-reviews.

Same, I have contacted Zenodo via both twitter and email to see what the issue is

from jose-reviews.

Hello human, I'm @whedon. I'm here to help you with some common editorial tasks. @shivupa, it looks like you're currently assigned as the reviewer for this paper 🎉.

⭐ Important ⭐

If you haven't already, you should seriously consider unsubscribing from GitHub notifications for this (https://github.com/openjournals/jose-reviews) repository. As a reviewer, you're probably currently watching this repository which means for GitHub's default behaviour you will receive notifications (emails) for all reviews 😿

To fix this do the following two things:

1. Set yourself as 'Not watching' https://github.com/openjournals/jose-reviews:

2. You may also like to change your default settings for this watching repositories in your GitHub profile here: https://github.com/settings/notifications

For a list of things I can do to help you, just type:

@whedon commands

from jose-reviews.

Attempting PDF compilation. Reticulating splines etc...

from jose-reviews.

--> Check article proof 📄 <--

from jose-reviews.

@whedon commands

from jose-reviews.

Here are some things you can ask me to do:

# List all of Whedon's capabilities
@whedon commands

# Assign a GitHub user as the sole reviewer of this submission
@whedon assign @username as reviewer

# Add a GitHub user to the reviewers of this submission
@whedon add @username as reviewer

# Remove a GitHub user from the reviewers of this submission
@whedon remove @username as reviewer

# List of editor GitHub usernames
@whedon list editors

# List of reviewers together with programming language preferences and domain expertise
@whedon list reviewers

# Change editorial assignment
@whedon assign @username as editor

# Set the software archive DOI at the top of the issue e.g.
@whedon set 10.0000/zenodo.00000 as archive

# Open the review issue
@whedon start review

🚧 🚧 🚧 Experimental Whedon features 🚧 🚧 🚧

# Compile the paper
@whedon generate pdf

from jose-reviews.

A few comments:

1. I really like the idea of running a demo through Jupyter, as it is convenient to test the code directly. I tried the cloud version for molecular_dynamics but not successful. Please correct me if I did it wrong. http://35.230.133.1/notebook/notebooks/examples/molecular_dynamics.ipynb#
   The error message is here:

TypeError                                 Traceback (most recent call last)
 in ()
----> 1 system = md_simulation(100, 273.15, 100, 5000, 50)
 in md_simulation(number_of_particles, temperature, box_length, number_of_steps, sample_frequency)
     16                                                                                                  system.cut_off)
     17         # Run the equations of motion integrator algorithm
---> 18         system.particles = md.velocity_verlet(system.particles, system.timestep_length, system.box_length)
     19         # Sample the thermodynamic and structural parameters of the system
     20         system = md.sample(system.particles, system.box_length, system.initial_particles, system)
TypeError: velocity_verlet() missing 1 required positional argument: 'cut_off' 

2. I tried to run it on a linux computer through a terminal, it was not successful either. I followed exactly all the steps shown in README.md. I suspected I missed installing some libraries. I think it will be beneficial if a detailed installation manual can be provided.

from jose-reviews.

Can you open this as an issue on the submission repo. I had some questions about this too.

from jose-reviews.

@shivupa I just opened an issue at arm61/pylj#11

from jose-reviews.

@shivupa, @TJFord -- the submitting author is also working on improved narrated examples for this package. We will wait for a heads-up that those are ready, and then please have a look at them, too.

from jose-reviews.

@labarba @shivupa it looks like the example monte carlo simulation is not working either. @arm61, can you double check it?
Also a few other minor suggestions:

1. would it be better if the title is changed to "pylj: A teaching tool for classical molecular dynamics simulation "? as the focus of the tool is on molecular dynamics simulation.
2. I would like to cite a few other widely used open source molecular dynamics simulation packages in the paper, such as lammps, gromacs, etc., for those students if they want to know more about it.

---------------------------------------------------------------------------
AttributeError                            Traceback (most recent call last)
 in ()
----> 1 system = mc_simulation(2, 273.15, 45, 100000, 100)

 in mc_simulation(number_of_particles, temperature, box_length, number_of_steps, sample_frequency)
      8     system.particles, system.distances, system.energies = comp.compute_energy(system.particles, 
      9                                                                               system.box_length,
---> 10                                                                               system.cut_off)
     11     old_energy = system.energies.sum()
     12     system = mc.sample(system.particles, system.box_length, old_energy, system)

AttributeError: 'System' object has no attribute 'cut_off'

from jose-reviews.

I agree that it should say "molecular simulation," or better "molecular dynamics simulation," everywhere.

Folks from outside the MD word will have no idea what the author means otherwise.

from jose-reviews.

I'd suggest "molecular simulation" since this encompasses molecular dynamics and Monte Carlo.

from jose-reviews.

Paper title has been updated in arm61/pylj@bb492be

from jose-reviews.

There are now two new examples of possible exercises showing the utility of pylj these can be found https://github.com/arm61/pylj/tree/master/examples . This means there is now the molecular dynamic and monte carlo examples showing how pylj can be used for teaching about simulation methods and the ideal gas example showing an application of pylj to build on traditional physical chemistry lecture material.

Note that to run these you need to build the most recent commit of pylj (this is because in developing the tutorials I recognised some more useful pedagogical methods that could be applied). This has also lead to the pairwise module which is a pure python implementation of the comp module (this is MUCH slower than the cython but possibly more pedagogically interesting).

I hope this takes the submission up to the expected level that is required. Sorry about the delay (life gets in the way sometimes).

from jose-reviews.

I have an observation that is optional for you to address, but I ask you to consider… You are using British spelling in the mc.initialise() function. This is likely to trip many if not most users! I see that you are based in Bath, so it make sense for your local users. Maybe you can overload the name and have two functions that do the same thing?

from jose-reviews.

This is a fair comment (especially considering the python preference for US english). I have mapped the functions in arm61/pylj@2697140

from jose-reviews.

@TJFord I have added references to the paper to gromacs, lammps and dlpoly in arm61/pylj@21baa24

from jose-reviews.

@whedon generate pdf

from jose-reviews.

Attempting PDF compilation. Reticulating splines etc...

from jose-reviews.

@TJFord I'm unsure how whedon works, but we may need to check all the boxes above to proceed.

from jose-reviews.

@labarba it seems that both reviewers are happy. I was wondering what the next stage was?

from jose-reviews.

@arm61 Could you try moving the positioning of your figure down, so that more text moves to the first page, under Summary? There's a big chunk of white space there.

from jose-reviews.

Editorial feedback

Summary:
(l.1) classical simulation—> classical atomistic simulation
(l.2) the citation should appear as Author, Year (here and elsewhere)
(l.4) define NVE and NVT
(l.5) capitalize Argon
(l.7) capitalize Matplotlib
(l.9) explaination—>explanation (typo)
This whole passage is hard to parse. The sentence construction is perhaps a bit ritzy. Can you simplify? Something like:
PyLJ is written in Python (using Cython for pairwise interactions) and uses Jupyter notebooks and Matplotlib for visualization (example below). It can be easily deployed in a computer laboratory, and students interact with it without needing to use the command line, as they would when using other molecular dynamics software like Gromacs, DL_poly or others. (Refs.) We provide example notebooks in the repository, showing how to use PyLJ to simulate a 2D gas system using molecular dynamics and Monte-Carlo methods. A variety of other applications are possible.
(p.2) “thermostating” is jargon: please define or rephrase

Statement of need:
classical simulation—> classical atomistic simulation
PyLJ allows this by offering—> PyLJ offers
capitalize Matplotlib
“In addition to use in the introduction of the simulation itself” —> huh? Please rephrase.
—.
Audience: you mention in the first sentence undergraduates in chemistry and physics. Can you say more about where in the curriculum this would be used? (what course? what year?)

from jose-reviews.

Hmm... I don't get why your first reference is displayed with the full title in the Summary. The citation looks correct in the paper.md file.

from jose-reviews.

Sorry to give you comments piecemeal… I did the first part on a flight yesterday.

You give a short description of the examples, with a link to the folder in the repo, within the “Statement of Need” section. It doesn’t quite belong there. You could use a new heading “Usage” and talk about the examples there. It would also be good to add a bulleted list of the examples in the README. Navigating down the folders to find the examples is not user-friendly, and since some folders have two notebooks, a user may not know where to start. I, myself, click through the various notebooks, and don’t know what order one is supposed to go through this.

What I’d like to see is a paragraph in the paper (and maybe in a new README in the examples folder) that orients an independent learner or instructor on getting started and adopting this tool. It took me many visits to your repository to finally stumble on the guide.ipynb file just now! Please add a link to this file from the README. I think the “How to teach with PyLJ” section here would be useful to have in the paper. (But please don’t use the un-semantic hyperlinked “here.”)

from jose-reviews.

I went to the Getting Started page on the documentation, and clicked on the Tutorial links, which launch a Binder session. However, they all give a "cannot resolve ref" error.

from jose-reviews.

@whedon generate pdf

from jose-reviews.

Attempting PDF compilation. Reticulating splines etc...

from jose-reviews.

--> Check article proof 📄 <--

from jose-reviews.

Maybe the problem with the citation to LENNARD-JONES is that the entry has no author field.

from jose-reviews.

Okay, I think that is everything that you've mentioned resolved. Hopefully, the paper is more suitable not (commits arm61/pylj@8d6ebd3 arm61/pylj@94a5a49 arm61/pylj@ddf306f arm61/pylj@22b926f). The only thing that you suggested that I haven't done is captalising argon, this is because (as I understand) it is IUPAC convention to not treat element names as proper nouns (https://chemistry.stackexchange.com/questions/6381/why-do-people-often-capitalize-element-names).

I have also added a mention of the examples in the README (commit arm61/pylj@6669515). I don't go into detail as my plan is to mention the paper once it is published which gives more details.

I have move the "How to teach with pylj" bit from the guide.ipynb file to the "Usage" section of the paper and as a result have decided that the guide.ipynb file is surplus to requirements.

Thanks for the thorough comments @labarba

from jose-reviews.

@whedon generate pdf

from jose-reviews.

Attempting PDF compilation. Reticulating splines etc...

from jose-reviews.

--> Check article proof 📄 <--

from jose-reviews.

Good changes!

Little fixes:

• you have punctuation before citations that refer to previous passage; better after
• string of citation for other packages: better to interleave, "Gromacs (ref), LAMMPS (ref), DLPOLY (ref)"
• typo: repoository -->repository
• typo: how to use pylj to simulated --> simulate
• typo: using eaither --> either
• run-on sentence: "…in a teaching laboratory setting, with this in mind we suggest" --> period or semi-colon after "setting" and comma after "mind"
• typo (p.2): folloiwng --> following
• period after "courses"

from jose-reviews.

Done! arm61/pylj@d5f0a23

from jose-reviews.

@whedon generate pdf

from jose-reviews.

Attempting PDF compilation. Reticulating splines etc...

from jose-reviews.

--> Check article proof 📄 <--

from jose-reviews.

OK! Now, please make a deposit on Zenodo or your favorite archive (or update a versioned DOI, if the case), and give me the DOI here.

from jose-reviews.

The DOI is 10.5281/zenodo.1312617

from jose-reviews.

@whedon set 10.5281/zenodo.1312617 as archive

from jose-reviews.

OK. 10.5281/zenodo.1312617 is the archive.

from jose-reviews.

Something is wrong with the Zenodo DOI you gave me
https://doi.org/10.5281/zenodo.1312617

"This DOI cannot be found in the DOI System"

Can you double check?

from jose-reviews.

that should be correct zenodo link. It might be the case that it takes an hour or two for the DOI that zenodo provides to mature? I created a new release so the DOI is very "fresh"

from jose-reviews.

OK. We'll wait. If the DOI does not resolve tomorrow morning your time (please check), then send an email to Zenodo help. I won't publish the JOSE paper until the DOI of the archive resolves.

from jose-reviews.

I mean, at the moment it is pushed to JOSE-papers, but I haven't made a Crossref deposit.

from jose-reviews.

I checked and the DOI still gives me an error: https://doi.org/10.5281/zenodo.1312617

from jose-reviews.

It is resolved now

from jose-reviews.

Crossref threw citation errors. Please fix DOI links for the Gromacs paper and Plimpton 1995.

I will have to do the whole process again!

from jose-reviews.

Resolved in arm61/pylj@ce91689

Sorry about that, apparently the bibtex served by sciencedirect incorrectly prepends "http://doi.org/" for the doi...

from jose-reviews.

🎉🎉🎉 Congratulations on your paper acceptance! 🎉🎉🎉

If you would like to include a link to your paper from your README use the following code snippets:

Markdown:
[![DOI](https://jose.theoj.org/papers/10.21105/jose.00019/status.svg)](https://doi.org/10.21105/jose.00019)

HTML:
<a style="border-width:0" href="https://doi.org/10.21105/jose.00019">
  <img src="https://jose.theoj.org/papers/10.21105/jose.00019/status.svg" alt="DOI badge" >
</a>

This is how it will look in your documentation:

We need your help!

Journal of Open Source Education is a community-run journal and relies upon volunteer effort. If you'd like to support us please consider doing either one (or both) of the the following:

• Volunteering to review for us sometime in the future. You can add your name to the reviewer list here: http://jose.theoj.org/reviewer-signup.html
• Making a small donation to support our running costs here: https://www.flipcause.com/secure/cause_pdetails/Mjk3ODA=

from jose-reviews.

@arm61 -- your paper is now published in JOSE ‼️ Congratulations 🎉
You can now add the happy green JOSE button on your repository.

@shivupa, @TJFord — Thank you again for your review 🙏

from jose-reviews.