finalize 1.0.0 #261
Merged
finalize 1.0.0 #261
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Member
orbeckst
commented
Oct 31, 2022
orbeckst
commented
- for release 1.0 #208
- CHANGES
- docs
- additional tests
- add tests - add deprecation docs - raise warnings at stacklevel 2 so that they appear for decorrelate() and friends
Codecov Report
@@ Coverage Diff @@
## master #261 +/- ##
==========================================
+ Coverage 98.65% 98.69% +0.03%
==========================================
Files 26 26
Lines 1711 1759 +48
Branches 352 363 +11
==========================================
+ Hits 1688 1736 +48
Misses 3 3
Partials 20 20
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. |
orbeckst
force-pushed
the
finalize-1.0.0
branch
from
9a3a57d to
fd37715
Compare
xiki-tempula
requested changes
Nov 1, 2022
| This function computes :math:`R_c` from `equation 16`_ from [Fan2021]_. | ||
| The code is modified based on Shujie Fan's (@VOD555) work. Zhiyi Wu | ||
| (@xiki-tempula) improved the performance of the original algorithm by | ||
| pre-computing block averages for fractional increments of `precision`. |
There was a problem hiding this comment.
Shujie Fan's (@VOD555) work has already implemented the pre-computing block averages. His work uses the normal python for loop and I have improved the performance by vectorise it using numpy array.
| """Convert a dhdl DataFrame to a series for subsampling. | ||
|
|
||
| The series is generated by summing over all energy components (axis 1 of | ||
| `df`), as for ``method='all'`` in :func:`u_nk2series`. Normally, `df` only | ||
| contains a single energy component. |
There was a problem hiding this comment.
I think it might not be very good to have Normally in
Normally,
dfonly contains a single energy component.
If one use split protocol in Gromacs, the dhdl will always have more than one column and using the phrase Normally might indicate that having more than one column in dHdl is abnormal. I think it might be good to change it to
Mostdf` only contains a single energy component.
|
Thank you for the clarifications, I will change the text. I’m very glad that you looked over everything carefully.
… Am 11/1/22 um 02:46 schrieb Zhiyi Wu ***@***.***>:
@xiki-tempula requested changes on this pull request.
Many thanks for finalising the release just some minor doc comments.
In src/alchemlyb/convergence/convergence.py:
> @@ -216,16 +223,25 @@ def fwdrev_cumavg_Rc(series, precision=0.01, tol=2):
8 3.042032 3.046597 0.9
9 3.044149 3.044385 1.0
- Note
- ----
- This function computes :math:`R_c` from equation 16 from
- https://www.ncbi.nlm.nih.gov/pmc/articles/PMC8397498/#FD16. The code is
- modified based on Shujie Fan's ***@***.***) work.
-
- Please cite [Fan2021]_ when using this function.
-
+ Notes
+ -----
+ This function computes :math:`R_c` from `equation 16`_ from [Fan2021]_.
+ The code is modified based on Shujie Fan's ***@***.***) work. Zhiyi Wu
+ ***@***.***) improved the performance of the original algorithm by
+ pre-computing block averages for fractional increments of `precision`.
Shujie Fan's ***@***.***) work has already implemented the pre-computing block averages. His work uses the normal python for loop and I have improved the performance by vectorise it using numpy array.
In src/alchemlyb/preprocessing/subsampling.py:
> """Convert a dhdl DataFrame to a series for subsampling.
+ The series is generated by summing over all energy components (axis 1 of
+ `df`), as for ``method='all'`` in :func:`u_nk2series`. Normally, `df` only
+ contains a single energy component.
I think it might not be very good to have Normally in
Normally, df only contains a single energy component.
If one use split protocol in Gromacs, the dhdl will always have more than one column and using the phrase Normally might indicate that having more than one column in dHdl is abnormal. I think it might be good to change it to
Most df` only contains a single energy component.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you authored the thread.
|
- finalize CHANGES (reformatting, consolidating of entries) - add @DrDomenicoMarson to contributors for 1.0.0 - add versionadded 1.0.0 to docs where necessary - CHANGES note pymbar 3.x for 1.0, see issue #205
- docs:
- Only list functions in convergence submodule.
- updated text
- added Fan 2020 reference
- fix reST
- add Klimovich citation for normal fwdbck convergence
- functions
- use *same* kwarg for tolerance: `tol` for Rc and Ac (changed
A_c `diff` to `tol`)
- update A_c/R_c docs to use `tol`
- explicitly set tol=2 in tests
- include everyone in AUTHORS - sort alphabetically - updated to most recent year
Note that sphinx is still complaining about duplicate object descriptions in workflows.base.WorkflowBase, namely any of the attributes.
orbeckst
force-pushed
the
finalize-1.0.0
branch
from
f8e5758 to
c57934a
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.