-
Notifications
You must be signed in to change notification settings - Fork 23
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
Issue 801 execution quickrun #814
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #814 +/- ##
==========================================
- Coverage 92.53% 92.31% -0.23%
==========================================
Files 133 133
Lines 9767 9767
==========================================
- Hits 9038 9016 -22
- Misses 729 751 +22
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, couple of typos and a question.
The **execution** of these simulations however requires a large amount of computational power and is intended to be | ||
distributed onto a HPC environment. | ||
To do this requires storing and sending the details of the simulation from the local workstation to a HPC environment, | ||
this can be done via the :func:`.Transformation.dump` function which creates a saved "json" version of the data. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should this really be here? It feels like this page should concentrate "I have a JSON, what do I do now", and this should get punted over to the setup section.. maybe we're missing a "writing out an alchemical network" section?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I added a link to the relevant cookbook here. It's duplicated information, but it's also essential for anything else on the page to make sense so it's probably warranted
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Two comments, but otherwise looks good to me.
The planning and preparation of a campaign of alchemical simulations using the ``openfe`` package is intended to be | ||
achievable on a local workstation in a matter of minutes. | ||
The **execution** of these simulations however requires a large amount of computational power and is intended to be | ||
distributed across a HPC environment. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's probably not necessary, but one thing here that might be a bit of a catch to a novice user is "o so I need a HPC machine to do this", maybe a note that says something like "you can also do this on your workstation" might be good to spell things out?
|
||
:: | ||
|
||
openfe quickrun transformation.json |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This doesn't use the -o
flag, but the next example does. Would it be worth at least making them equivalent? (and saying that -o means that you're generating an output file named results.json)?
fixes issue #801
Co-authored-by: Irfan Alibay <[email protected]>
clarify can run single sim locally homogenise quickrun examples add link to working with results page
8b012fe
to
139e9c3
Compare
fixes #801
Checklist
news
entryDevelopers certificate of origin