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

Corrected the instructions for running in parallel #1709

Merged
merged 1 commit into from
Sep 26, 2019
Merged

Conversation

rwest
Copy link
Member

@rwest rwest commented Aug 27, 2019

The option -n $Processes is an argument to rmg.py not to python.

Motivation or Problem

The documentation suggested python -n $Processes path/to/rmg.py path/to/input.py

Description of Changes

It now says python path/to/rmg.py -n $Processes path/to/input.py

@xiaoruiDong xiaoruiDong requested a review from ajocher August 27, 2019 19:13
ajocher
ajocher previously approved these changes Aug 27, 2019
Copy link
Contributor

@ajocher ajocher left a comment

Choose a reason for hiding this comment

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

Ran superminimal and minimal examples with the updated documentation and both examples worked as expected with multiprocessing.

@mliu49
Copy link
Contributor

mliu49 commented Aug 28, 2019

Please don't merge yet! I haven't decided exactly how we'll handle the PRs which have been opened since Py3 stuff started.

@rwest
Copy link
Member Author

rwest commented Aug 28, 2019

Right.
I presume it can be similar to ones that were already open but not merged? I.e we'll rebase them. But perhaps pick some bugfix ones to backport if we have a maintenance branch of RMG 2.4 for a little while?
Anyway, needn't be discussed here

@codecov
Copy link

codecov bot commented Sep 26, 2019

Codecov Report

Merging #1709 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #1709   +/-   ##
=======================================
  Coverage   32.65%   32.65%           
=======================================
  Files          87       87           
  Lines       26158    26158           
  Branches     6874     6874           
=======================================
  Hits         8541     8541           
+ Misses      16656    16645   -11     
- Partials      961      972   +11
Impacted Files Coverage Δ
rmgpy/data/statmech.py 42.39% <0%> (ø) ⬆️
rmgpy/rmg/pdep.py 12.21% <0%> (ø) ⬆️
rmgpy/reaction.py 0% <0%> (ø) ⬆️
rmgpy/data/kinetics/family.py 48.49% <0%> (ø) ⬆️
rmgpy/yml.py 15.71% <0%> (ø) ⬆️
rmgpy/data/kinetics/database.py 50.73% <0%> (ø) ⬆️
rmgpy/rmg/input.py 34.34% <0%> (ø) ⬆️
rmgpy/molecule/molecule.py 0% <0%> (ø) ⬆️
rmgpy/statmech/ndTorsions.py 59.89% <0%> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 581ca34...59bfa33. Read the comment docs.

@amarkpayne
Copy link
Member

@ajocher if this looks good to you then you can merge this in as soon as the tests pass.

@amarkpayne
Copy link
Member

@mliu49 I have also cherry-picked and pushed this to the v2.5.0 branch as it should be included in our legacy python 2 branch

ajocher
ajocher previously approved these changes Sep 26, 2019
The option `-n $Processes` is an argument to `rmg.py` not to `python`.
@amarkpayne amarkpayne merged commit 788c9c7 into master Sep 26, 2019
@amarkpayne amarkpayne deleted the parallel_docs branch September 26, 2019 20:08
@mliu49 mliu49 mentioned this pull request Dec 16, 2019
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.

4 participants