Skip to content

Corrected the instructions for running in parallel#1709

Merged
amarkpayne merged 1 commit intomasterfrom
parallel_docs
Sep 26, 2019
Merged

Corrected the instructions for running in parallel#1709
amarkpayne merged 1 commit intomasterfrom
parallel_docs

Conversation

@rwest
Copy link
Copy Markdown
Member

@rwest rwest commented Aug 27, 2019 •
edited
Loading

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
View reviewed changes
Copy link
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown

codecov bot commented Sep 26, 2019 •
edited
Loading

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
Copy Markdown
Member

amarkpayne commented Sep 26, 2019

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

@amarkpayne
Copy link
Copy Markdown
Member

amarkpayne commented Sep 26, 2019

@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
View reviewed changes
@rwest @amarkpayne
Corrected the instructions for running in parallel
59bfa33 
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 Nov 26, 2019
Closed
@mliu49 mliu49 mentioned this pull request Dec 16, 2019
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@ajocher ajocher ajocher approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@rwest @mliu49 @amarkpayne @ajocher