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.

Sign up for GitHub

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

Jump to bottom

clear up language a bit for dg report help text #549

Merged
merged 6 commits into from
Oct 11, 2023
Merged

clear up language a bit for dg report help text #549

Changes from 3 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
bd3cd51
clear up language a bit
mikemhenry Sep 8, 2023
09c33db
Merge branch 'main' into feat/tweak_help_msg_for_gather
IAlibay Oct 9, 2023
13992d7
Update gather.py
IAlibay Oct 9, 2023
b84b87c
Merge branch 'main' into feat/tweak_help_msg_for_gather
IAlibay Oct 11, 2023
9a10ac7
Update gather.py
IAlibay Oct 11, 2023
a501dcd
oxford comma
IAlibay Oct 11, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 5 additions & 4 deletions openfecli/commands/gather.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -230,11 +230,12 @@ def gather(rootdir, output, report):
The results reported depend on ``--report`` flag:

\b
* 'dg' (default) reports the ligand and the results are the maximum
likelihood estimate of its absolute free, and the uncertainty in
that.
* 'dg' (default) reports the ligand and the results as the maximum
Copy link
Member

Choose a reason for hiding this comment

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

Sorry @mikemhenry I overwrote this a bit, totally agreed it needed a change, just thought it might need a bit of an expansion on what you wrote.

@hannahbaumann does this seem sensible to you?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, I think this generally sounds good, I was just a little bit confused about the "likelihood estimate of its absolute free" part and also the comma before ,and makes it look a little bit like the "from DDG replica averages and standard deviations" only belongs to the "uncertainty" part.
Maybe something like:
"* 'dg' (default) reports the ligand and its absolute free energy and the associated uncertainty as the maximum likelihood estimate obtained from DDG replica averages and standard deviations." Or maybe that's even more confusing, I'm not sure =)

Copy link
Member

Choose a reason for hiding this comment

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

thanks @hannahbaumann ! I've updated it accordingly

likelihood estimate of the replica average absolute free energy,
and the associated uncertainty based on the standard deviation
between replciates.
* 'ddg' reports pairs of ligand_i and ligand_j, the calculated
relative free energy DDG(i->j) = DG(j) - DG(i) and its uncertainty
relative free energy DDG(i->j) = DG(j) - DG(i) and its uncertainty.
* 'dg-raw' reports the raw results, giving the leg (vacuum, solvent, or
complex), ligand_i, ligand_j, the raw DG(i->j) associated with it.

Expand Down
Loading