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 more descriptions to tutorials, update links to base code #95

Merged
merged 7 commits into from
Nov 14, 2023

Conversation

marjanalbooyeh
Copy link
Collaborator

This PR addresses some comments in this JOSS review.

Comments

  • In the tutorials, the authors repeatedly refer the user to the source code of the different classes for examples (i.e. Pack and Lattice). That should not be the case. Examples of using the different classes should be available in the documentation or the tutorial.
  • In tutorial 2, The authors provide examples of how to create a custom molecule. In example 4, they mentioned that a custom molecule can be defined through a class. The authors should give a concrete example of the required attributes of that class and how to define the molecule instead of just referring to the source code.
  • I found it very good that you can define your force field using .xml files. However, I think the authors need to specify how the .xml file should be constructed or re-direct the interested user to a reference with that information.

Fix:

  • Updated the text and added more description/instructions on subclasses.
  • I added links to the flowemd documentation replacing links to the code base.
  • Added a link to foyer xml sample format

Copy link

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.


Powered by ReviewNB

@chrisjonesBSU chrisjonesBSU linked an issue Nov 14, 2023 that may be closed by this pull request
3 tasks
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 @marjanAlbouye

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Tutorial Fixes
2 participants