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

Update required version of rdkit to avoid severe memory leaks #1851

Merged
merged 1 commit into from Dec 16, 2019

Conversation

kspieks
Copy link
Contributor

@kspieks kspieks commented Dec 13, 2019

Motivation or Problem

Significant memory leaks have been observed when using an rmg_env containing rdkit version 2019.03.4.0 as referenced in issue #1850. The issue is documented on the rdkit page rdkit/rdkit#2639

Memory leaks are not observed when using a more recent rdkit version of 2019.09.1.0 (and it looks like they've already released a version 2019.09.2.0)

Description of Changes

Updated the minimum required version number for rdkit in the environment.yml file

Testing

I used this environment.yml file to create an rmg_env. Doing conda list rdkit produced the following output, confirming that the change is implemented correctly

# Name                    Version                   Build  Channel
rdkit                     2019.09.2.0      py37h65625ec_1    rdkit

@codecov
Copy link

codecov bot commented Dec 13, 2019

Codecov Report

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

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #1851   +/-   ##
=======================================
  Coverage   44.23%   44.23%           
=======================================
  Files          83       83           
  Lines       21539    21539           
  Branches     5645     5645           
=======================================
  Hits         9527     9527           
+ Misses      10956    10942   -14     
- Partials     1056     1070   +14
Impacted Files Coverage Δ
rmgpy/data/statmech.py 42.2% <0%> (ø) ⬆️
rmgpy/rmg/pdep.py 12.21% <0%> (ø) ⬆️
arkane/kinetics.py 12.14% <0%> (ø) ⬆️
rmgpy/data/kinetics/family.py 49.06% <0%> (ø) ⬆️
rmgpy/statmech/ndTorsions.py 59.78% <0%> (ø) ⬆️
rmgpy/yml.py 15.71% <0%> (ø) ⬆️
arkane/sensitivity.py 10% <0%> (ø) ⬆️
rmgpy/data/kinetics/database.py 50.61% <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 150aec0...598e2eb. Read the comment docs.

@alongd
Copy link
Member

alongd commented Dec 16, 2019

The updated Linux x64 version is v2019.09.2.0

@mliu49
Copy link
Contributor

mliu49 commented Dec 16, 2019

I think this looks good. Alon, were you suggesting any changes, or confirming that this is ok?

@alongd
Copy link
Member

alongd commented Dec 16, 2019

Suggesting to change 2019.09.1.0 into 2019.09.2.0

@mliu49
Copy link
Contributor

mliu49 commented Dec 16, 2019

Ah, ok. I think this is fine though, since 2019.09.1.0 is the release which fixes the issue, so anything newer is acceptable.

@mliu49
Copy link
Contributor

mliu49 commented Dec 16, 2019

Of course, unless there are other reasons that 2019.09.1.0 does not work for us, in which case we can increment it to 2019.09.2.0.

@kspieks
Copy link
Contributor Author

kspieks commented Dec 16, 2019

Of course, unless there are other reasons that 2019.09.1.0 does not work for us, in which case we can increment it to 2019.09.2.0.

Both versions have not had memory leaks for me, so I think this PR is good to go

@mliu49 mliu49 merged commit d7dee79 into master Dec 16, 2019
@mliu49 mliu49 deleted the update_rdkit_version_in_yml branch December 16, 2019 20:21
This was referenced 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.

None yet

3 participants