Add gomc parser #78

msoroush · 2019-04-30T21:05:09Z

Add GOMC parser to alchemlyb. Energy output file for Free Energy Calculation is similar to GROMCAS. This parser, reads, Total energy, PV, derivative of energy (for current lambda state), and change of energy (between current lambda state and all other lambda states).

codecov-io · 2019-04-30T21:10:00Z

Codecov Report

Merging #78 into master will decrease coverage by 0.88%.
The diff coverage is 91.91%.

@@            Coverage Diff             @@
##           master      #78      +/-   ##
==========================================
- Coverage   98.16%   97.27%   -0.89%     
==========================================
  Files          11       12       +1     
  Lines         599      698      +99     
  Branches      116      141      +25     
==========================================
+ Hits          588      679      +91     
- Misses          4        5       +1     
- Partials        7       14       +7

Impacted Files	Coverage Δ
src/alchemlyb/parsing/gomc.py	`91.91% <91.91%> (ø)`

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 86d2d26...325dc3d. Read the comment docs.

dotsdl · 2019-05-02T13:42:00Z

@msoroush thanks for opening this PR! I've scheduled it for review on 2019.05.05.

orbeckst · 2019-05-03T16:48:44Z

You'll need tests.

For tests you need to add a data set to alchemtest, so open an issue/PR over there, please. See https://github.com/alchemistry/alchemtest/wiki/contributing

msoroush · 2019-05-03T17:04:34Z

You'll need tests.

For tests you need to add a data set to alchemtest, so open an issue/PR over there, please. See https://github.com/alchemistry/alchemtest/wiki/contributing

I am still validating my implementation in GOMC. Once it validate it, I will create the test. I might cancel this PR because I noticed that slicing function does not work index name other than time.

orbeckst · 2019-05-03T17:10:21Z

See #79 (comment) for comments on slicing.

orbeckst · 2019-05-03T17:17:09Z

Btw, you don't have to close the PR. You can keep it open and update it.

Thanks for offering to contribute. As I said, it would be good to have code to deal with MC data. We just have to make sure that the code remains maintainable and one of the key solutions there is to do testing really well.

Please also be aware that there's no-one being paid to do development on alchemlyb so any code reviews or comments might take some time. We are all way to busy and we try to carve out some time to move this project forward. So please have some patience with us. It's ok to ping someone with their GitHub handle @-mention (eg @orbeckst for me) after a few days if there follow-up seems to be missing.

msoroush · 2019-05-03T17:28:41Z

Please also be aware that there's no-one being paid to do development on alchemlyb so any code reviews or comments might take some time. We are all way to busy and we try to carve out some time to move this project forward. So please have some patience with us. It's ok to ping someone with their GitHub handle @-mention (eg @orbeckst for me) after a few days if there follow-up seems to be missing.

I will keep it open and work on it. I tried to keep the file format to be similar to GROMACS to make everyones life easy. I did not want to write analysis tools from scratch, so I started to use alchemical-analysis and then alchemlyb. This is a new field for me and I am trying to learn it as fast as possible. Thats why I have too many questions.

…g module.

dotsdl

Looking very good so far @msoroush. I'll second @orbeckst's statement that for new parsers we'll need a corresponding dataset added to alchemtest. I'll also echo that this project ratchets forward slowly with an eye toward producing correct results for everything we release, so bear with us. We definitely want to support as many formats for alchemical free energy data as we can, and this is no exception.

Please feel free to push to this PR as you improve your parser. When ready, also open a PR on alchemtest for a dataset that works well for testing. You'll want to produce a dataset that is substantial enough to yield testing value but small enough to not balloon the testing package (shoot for less than 10MB if you can; definitely use e.g. bzip2 compression for individual files).

Thanks for your patience. Looking forward to reviewing more as it comes!

src/alchemlyb/parsing/gomc.py

msoroush · 2019-07-02T18:13:37Z

You'll need tests.

For tests you need to add a data set to alchemtest, so open an issue/PR over there, please. See https://github.com/alchemistry/alchemtest/wiki/contributing

Does it matter which forcefield or water model I am using to generate benzene solvation data set?

orbeckst · 2019-07-02T21:29:37Z

No, ideally it should be something publicly available. You should open an issue and PR in alchemtest and link it to this one by including #78 in the description.

orbeckst · 2019-07-02T21:30:27Z

See contributing to alchemtest for guide lines. Ask if you have questions.

- close alchemistry#83 - close alchemistry#84 - files were manually created based on the history (git log --format="format:%an %ad" --date="format:%Y" 0.1.0..HEAD | sort | uniq) and the merged PRs and closed issues

* Add GOMC data sets (for alchemistry/alchemlyb#78) * update the documentation * Change the file formatting. Store the all free energy files in inWater directory instead of storing in separate VDW and Coulomb directory. * compress the free energy files * close #33

orbeckst

@msoroush now that PR alchemistry/alchemtest#34 was merged, can you please add tests for

parsing
calculating free energies: you have both FEP and TI parsers so you should also calculate your solvation free energy with BAR/MBAR and TI

msoroush · 2019-07-19T17:34:43Z

can you please add tests for

parsing

calculating free energies: you have both FEP and TI parsers so you should also calculate your
solvation free energy with BAR/MBAR and TI

Where should I add test for parsing and calculating free energies?

orbeckst · 2019-07-19T17:38:28Z

Tests for parsing: new file test_gomc.py under src/alchemlyb/tests/parsing.
Tests for FEP (BAR/MBAR – your choice): src/alchemlyb/tests/test_fep_estimators.py
Tests for TI: src/alchemlyb/tests/test_ti_estimators.py

Have a look at the existing tests and model yours after them.

msoroush · 2019-07-22T17:09:13Z

@orbeckst I added the tests for GOMC parser and free energy estimator (BAR, MBAR, and TI). Please let me know If you want me to add additional test for GOMC parser?

I noticed that I would not load benzene data from GOMC datasets. I fixed the access.py file alchemistry/alchemtest#36.

…arser

orbeckst · 2019-07-26T18:40:50Z

No need to update api_proposal – it's the general ideas and outline. Thanks for checking.

orbeckst · 2019-07-26T18:45:07Z

The codecov status is missing because the upload from Travis is failing

Error: HTTPSConnectionPool(host='codecov.io', port=443): Max retries exceeded with url: /codecov/v4/raw/2019-07-26/4CA02E04D860053FE833B077F7D9C963/98bf9291d28dc957b5e82407d222dbcd847eb2c6/b3be7e92-5b6c-4e13-a9e6-223a1411d0b2.txt?AWSAccessKeyId=AKIAIHLZSCQCS4WIHD4A&Expires=1564165667&Signature=ouAwWnD%2FfiB%2FP0u7%2Bidshrlg%2Bqk%3D (Caused by NewConnectionError('<urllib3.connection.VerifiedHTTPSConnection object at 0x7fb6260d7da0>: Failed to establish a new connection: [Errno 110] Connection timed out'))

Not sure why.

orbeckst · 2019-07-26T18:54:25Z

@dotsdl can you have a brief look at this PR and if your requirements were addressed? It seems good to go.

dotsdl · 2019-07-26T19:50:20Z

I should be able to review this PR before Sunday. Thanks @msoroush for pushing this through to the finish line, and @orbeckst for shepherding it forward.

dotsdl · 2019-07-26T19:55:21Z

Went ahead and restarted the build; we'll see if Travis succeeds in shipping to codecov this time.

orbeckst · 2019-07-26T20:13:20Z

Nope – somethings is screwy with codecov. For some bizarre reason it lists the coverage for this PR as our project's coverage – see https://codecov.io/gh/alchemistry/alchemlyb. Maybe it gets confused because it comes from the master branch of a fork????

Anyway, we can revisit once this PR is merged.

orbeckst · 2019-07-26T20:23:37Z

I manually run the coverage

py.test --cov alchemlyb src/alchemlyb/tests
coverage html

with the following result:

============================= test session starts ==============================
platform darwin -- Python 3.6.7, pytest-3.2.3, py-1.4.34, pluggy-0.4.0
rootdir: /Volumes/Data/oliver/Biop/Projects/Methods/FreeEnergy/alchemlyb, inifile:
plugins: xdist-1.20.1, forked-0.2, cov-2.5.1
collected 146 items

src/alchemlyb/tests/test_fep_estimators.py .....................
src/alchemlyb/tests/test_import.py .
src/alchemlyb/tests/test_preprocessing.py ................................
src/alchemlyb/tests/test_ti_estimators.py ...........
src/alchemlyb/tests/test_version.py ..
src/alchemlyb/tests/parsing/test_amber.py ................................................................
src/alchemlyb/tests/parsing/test_gmx.py ..........
src/alchemlyb/tests/parsing/test_gomc.py ..
src/alchemlyb/tests/parsing/test_namd.py ..
src/alchemlyb/tests/parsing/test_util.py .

---------- coverage: platform darwin, python 3.6.7-final-0 -----------
Name                                         Stmts   Miss Branch BrPart  Cover
------------------------------------------------------------------------------
src/alchemlyb/__init__.py                        3      0      0      0   100%
src/alchemlyb/convergence/__init__.py            0      0      0      0   100%
src/alchemlyb/convergence/convergence.py         0      0      0      0   100%
src/alchemlyb/convergence/pade.py                0      0      0      0   100%
src/alchemlyb/estimators/__init__.py             3      0      0      0   100%
src/alchemlyb/estimators/bar_.py                41      0     10      0   100%
src/alchemlyb/estimators/mbar_.py               26      1      4      0    97%
src/alchemlyb/estimators/ti_.py                 31      0      4      0   100%
src/alchemlyb/parsing/__init__.py                0      0      0      0   100%
src/alchemlyb/parsing/amber.py                 235      2     98      4    98%
src/alchemlyb/parsing/gmx.py                   156      1     90      3    98%
src/alchemlyb/parsing/gomc.py                  105      5     48      7    92%
src/alchemlyb/parsing/namd.py                   32      0      8      0   100%
src/alchemlyb/parsing/util.py                   25      4      2      0    85%
src/alchemlyb/preprocessing/__init__.py          4      0      0      0   100%
src/alchemlyb/preprocessing/subsampling.py      43      0     16      0   100%
------------------------------------------------------------------------------
TOTAL                                          704     13    280     14    97%

The html pages are attached as coverage.zip; note that the current codecov looks similar, e.g., for alchemlyb/parsing/gomc.py.

I hope that helps @dotsdl in his review, even in the absence of codecov.

Based on these results I'll add my own comments in a moment.

orbeckst · 2019-07-26T20:28:25Z

The 85% coverage in parsing/util.py is harmless – it just needs to be tested under Py2, too.

The lack of testing of a number of except statements in gomc.py is an issue, though, it think these lines were copied from the gmx parser without further thinking. I'll make comments in the code.

orbeckst

See issues based on the coverage analysis.

src/alchemlyb/parsing/gomc.py

orbeckst · 2019-07-26T22:47:14Z

Looking good:

(alchemlyb) yngvi:alchemlyb oliver$ py.test --cov alchemlyb src/alchemlyb/tests
============================================================ test session starts ============================================================
platform darwin -- Python 3.6.7, pytest-3.2.3, py-1.4.34, pluggy-0.4.0
rootdir: /Volumes/Data/oliver/Biop/Projects/Methods/FreeEnergy/alchemlyb, inifile:
plugins: xdist-1.20.1, forked-0.2, cov-2.5.1
collected 146 items

src/alchemlyb/tests/test_fep_estimators.py .....................
src/alchemlyb/tests/test_import.py .
src/alchemlyb/tests/test_preprocessing.py ................................
src/alchemlyb/tests/test_ti_estimators.py ...........
src/alchemlyb/tests/test_version.py ..
src/alchemlyb/tests/parsing/test_amber.py ................................................................
src/alchemlyb/tests/parsing/test_gmx.py ..........
src/alchemlyb/tests/parsing/test_gomc.py ..
src/alchemlyb/tests/parsing/test_namd.py ..
src/alchemlyb/tests/parsing/test_util.py .

---------- coverage: platform darwin, python 3.6.7-final-0 -----------
Name                                         Stmts   Miss Branch BrPart  Cover
------------------------------------------------------------------------------
src/alchemlyb/__init__.py                        3      0      0      0   100%
src/alchemlyb/convergence/__init__.py            0      0      0      0   100%
src/alchemlyb/convergence/convergence.py         0      0      0      0   100%
src/alchemlyb/convergence/pade.py                0      0      0      0   100%
src/alchemlyb/estimators/__init__.py             3      0      0      0   100%
src/alchemlyb/estimators/bar_.py                41      0     10      0   100%
src/alchemlyb/estimators/mbar_.py               26      1      4      0    97%
src/alchemlyb/estimators/ti_.py                 31      0      4      0   100%
src/alchemlyb/parsing/__init__.py                0      0      0      0   100%
src/alchemlyb/parsing/amber.py                 235      2     98      4    98%
src/alchemlyb/parsing/gmx.py                   156      1     90      3    98%
src/alchemlyb/parsing/gomc.py                   98      1     48      7    95%
src/alchemlyb/parsing/namd.py                   32      0      8      0   100%
src/alchemlyb/parsing/util.py                   25      4      2      0    85%
src/alchemlyb/preprocessing/__init__.py          4      0      0      0   100%
src/alchemlyb/preprocessing/subsampling.py      43      0     16      0   100%
------------------------------------------------------------------------------
TOTAL                                          697      9    280     14    98%


======================================================= 146 passed in 142.37 seconds ========================================================

dotsdl · 2019-07-28T21:53:31Z

I've made some small changes, including adding -lambda to index names for the u_nk parser.

One note as an optimization: it might speed up dataframe parsing to use the pandas.read_csv parser for the data rows as we did for Gromacs, since this parser can be quite a bit faster than a parser done at the Python interpreter level (it is written in C). This implementation is great as a first pass, however; we are happy to have it.

Thanks @msoroush for sticking it out through this process!

@orbeckst, looks like we lost a little test coverage(?), but it's not clear to me where it could really be improved on this file. My local test run shows it has 95% coverage. Please merge when satsified; thanks!

orbeckst · 2019-07-29T19:37:47Z

Thanks. Because PR #85 was merged here and has not been merged into master, this PR has to wait until PR #85 is officially merged.

orbeckst · 2019-07-29T19:41:14Z

I also think we'll have to live with the coverage. I'll squash merge once PR #85 has been merged.

Thanks everyone!

msoroush · 2019-07-30T14:04:54Z

Thank you @dotsdl @orbeckst for all your help.

orbeckst · 2019-07-30T17:57:10Z

Congratulations @msoroush , your first PR in alchemlyb was merged. Thank you!

Add gomc parser

337f342

dotsdl self-assigned this May 2, 2019

This was referenced May 3, 2019

GOMC parser #77

Closed

Subsampling #79

Closed

Use time as index name and column name to be able to use preprocessin…

dcd3052

…g module.

dotsdl requested changes May 6, 2019

View reviewed changes

src/alchemlyb/parsing/gomc.py Outdated Show resolved Hide resolved

Removing the _extract_legend function.

6b7e825

This was referenced Jul 9, 2019

Adding GOMC free energy data sets alchemistry/alchemtest#33

Closed

Adding GOMC free energy data sets alchemistry/alchemtest#34

Merged

add AUTHORS and CHANGES

f90bb42

- close alchemistry#83 - close alchemistry#84 - files were manually created based on the history (git log --format="format:%an %ad" --date="format:%Y" 0.1.0..HEAD | sort | uniq) and the merged PRs and closed issues

Merge branch 'master' into master

a6b42bb

orbeckst requested changes Jul 19, 2019

View reviewed changes

msoroush added 3 commits July 22, 2019 11:42

Add parsing test for GOMC

77d0a74

Merge branch 'master' of https://github.com/msoroush/alchemlyb

3229455

Add test for GOMC parser, TI, BAR, and MBAR estimator

47b4585

Fix the missing parentheses in test_ti_estimators

45bcb81

Update the parsing documentation. Update the variable names in gomc p…

98bf929

…arser

orbeckst approved these changes Jul 26, 2019

View reviewed changes

orbeckst mentioned this pull request Jul 26, 2019

Removed redundant duplicate removal from gmx u_nk parser #87

Merged

Merge branch 'master' into master

a38807e

orbeckst requested changes Jul 26, 2019

View reviewed changes

src/alchemlyb/parsing/gomc.py Outdated Show resolved Hide resolved

src/alchemlyb/parsing/gomc.py Outdated Show resolved Hide resolved

src/alchemlyb/parsing/gomc.py Outdated Show resolved Hide resolved

src/alchemlyb/parsing/gomc.py Show resolved Hide resolved

msoroush added 2 commits July 26, 2019 17:53

Update gomc parser

34468d4

Merge branch 'master' of https://github.com/msoroush/alchemlyb

7df403c

orbeckst approved these changes Jul 26, 2019

View reviewed changes

dotsdl added 4 commits July 27, 2019 18:58

Small tweaks to gomc parser; no major changes

6eb966b

Merge remote-tracking branch 'msoroush/master' into msoroush-master

2c93819

Updated test to match *-lambda index names

f71d779

Merge branch 'master' into msoroush-master

72fe4f4

dotsdl approved these changes Jul 28, 2019

View reviewed changes

orbeckst added this to the release 0.2.0 milestone Jul 29, 2019

Merge branch 'master' into master

325dc3d

orbeckst merged commit c90bb88 into alchemistry:master Jul 30, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add gomc parser #78

Add gomc parser #78

msoroush commented Apr 30, 2019

codecov-io commented Apr 30, 2019 •

edited by codecov bot

Loading

dotsdl commented May 2, 2019

orbeckst commented May 3, 2019

msoroush commented May 3, 2019

orbeckst commented May 3, 2019 •

edited

Loading

orbeckst commented May 3, 2019

msoroush commented May 3, 2019

dotsdl left a comment

msoroush commented Jul 2, 2019

orbeckst commented Jul 2, 2019

orbeckst commented Jul 2, 2019

orbeckst left a comment

msoroush commented Jul 19, 2019

orbeckst commented Jul 19, 2019

msoroush commented Jul 22, 2019

orbeckst commented Jul 26, 2019

orbeckst commented Jul 26, 2019

orbeckst commented Jul 26, 2019

dotsdl commented Jul 26, 2019

dotsdl commented Jul 26, 2019

orbeckst commented Jul 26, 2019

orbeckst commented Jul 26, 2019 •

edited

Loading

orbeckst commented Jul 26, 2019

orbeckst left a comment

orbeckst commented Jul 26, 2019

dotsdl commented Jul 28, 2019

orbeckst commented Jul 29, 2019

orbeckst commented Jul 29, 2019 •

edited

Loading

msoroush commented Jul 30, 2019

orbeckst commented Jul 30, 2019

Add gomc parser #78

Add gomc parser #78

Conversation

msoroush commented Apr 30, 2019

codecov-io commented Apr 30, 2019 • edited by codecov bot Loading

Codecov Report

dotsdl commented May 2, 2019

orbeckst commented May 3, 2019

msoroush commented May 3, 2019

orbeckst commented May 3, 2019 • edited Loading

orbeckst commented May 3, 2019

msoroush commented May 3, 2019

dotsdl left a comment

Choose a reason for hiding this comment

msoroush commented Jul 2, 2019

orbeckst commented Jul 2, 2019

orbeckst commented Jul 2, 2019

orbeckst left a comment

Choose a reason for hiding this comment

msoroush commented Jul 19, 2019

orbeckst commented Jul 19, 2019

msoroush commented Jul 22, 2019

orbeckst commented Jul 26, 2019

orbeckst commented Jul 26, 2019

orbeckst commented Jul 26, 2019

dotsdl commented Jul 26, 2019

dotsdl commented Jul 26, 2019

orbeckst commented Jul 26, 2019

orbeckst commented Jul 26, 2019 • edited Loading

orbeckst commented Jul 26, 2019

orbeckst left a comment

Choose a reason for hiding this comment

orbeckst commented Jul 26, 2019

dotsdl commented Jul 28, 2019

orbeckst commented Jul 29, 2019

orbeckst commented Jul 29, 2019 • edited Loading

msoroush commented Jul 30, 2019

orbeckst commented Jul 30, 2019

codecov-io commented Apr 30, 2019 •

edited by codecov bot

Loading

orbeckst commented May 3, 2019 •

edited

Loading

orbeckst commented Jul 26, 2019 •

edited

Loading

orbeckst commented Jul 29, 2019 •

edited

Loading