Skip to content

Core concepts - #162

Merged
IAlibay merged 9 commits into
mainfrom
core_concepts
Oct 8, 2025
Merged

Core concepts#162
IAlibay merged 9 commits into
mainfrom
core_concepts

Conversation

@jthorton

@jthorton jthorton commented Sep 25, 2025

Copy link
Copy Markdown
Contributor

Description

Provide a brief description of the PR's purpose here.
First pass at the core concepts planned in #161

Todos

Notable points that this PR has either accomplished or will accomplish.

  • FEMap
  • Estimators
  • Plotting

Questions

  • Question1

Checklist

  • Added a news entry for new features, bug fixes, or other user facing changes.

Status

  • Ready to go

Tips

  • Comment "pre-commit.ci autofix" to have pre-commit.ci atomically format your PR.
    Since this will create a commit, it is best to make this comment when you are finished with your work.

@codecov

codecov Bot commented Sep 25, 2025

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 66.78%. Comparing base (45fe4b1) to head (ec82569).
⚠️ Report is 5 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #162      +/-   ##
==========================================
- Coverage   69.79%   66.78%   -3.02%     
==========================================
  Files          17       17              
  Lines        1096     1096              
==========================================
- Hits          765      732      -33     
- Misses        331      364      +33     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@IAlibay IAlibay left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Overall this looks good. Few suggestions / edits.

Comment thread docs/concepts/estimators.rst Outdated
Comment thread docs/concepts/estimators.rst
Comment thread docs/concepts/femap.rst Outdated
Comment thread docs/concepts/femap.rst Outdated
Comment thread docs/concepts/femap.rst Outdated
Comment thread docs/concepts/plotting.rst Outdated
Comment thread docs/concepts/plotting.rst Outdated
Absolute free energy plots compare calculated and experimental :math:`\Delta G` values for each ligand. Absolute
:math:`\Delta G` predictions can be obtained from a connected network of relative free energies. They therefore accumulate errors from multipule aspects of the calculation, including
edge accuracy, uncertainty quantification, perutration network design and estimator, giving a complete picture of protocol performance.
These plots are then useful for both method developers and users, as they show how well the overall ranking of ligands is predicted.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
These plots are then useful for both method developers and users, as they show how well the overall ranking of ligands is predicted.
These plots are useful for both method developers and users, as they show how well the overall ranking of ligands is predicted.

This sentence is more important than the previous one. I would have it take precedence maybe?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Same as above, this was resolved without action/comment, just checking if that was intended.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Strange the order has updated but the text update was lost this should be done now!

Comment thread docs/concepts/plotting.rst Outdated
Comment thread docs/concepts/plotting.rst Outdated
Comment thread docs/concepts/plotting.rst Outdated
jthorton and others added 6 commits October 7, 2025 16:23
Co-authored-by: Irfan Alibay <IAlibay@users.noreply.github.com>
Co-authored-by: Irfan Alibay <IAlibay@users.noreply.github.com>
Co-authored-by: Irfan Alibay <IAlibay@users.noreply.github.com>
Co-authored-by: Irfan Alibay <IAlibay@users.noreply.github.com>
Co-authored-by: Irfan Alibay <IAlibay@users.noreply.github.com>
@jthorton
jthorton requested a review from IAlibay October 8, 2025 10:01
@jthorton

jthorton commented Oct 8, 2025

Copy link
Copy Markdown
Contributor Author

Looks like the theme has updated! Looks cool, but I don't think it is clear what is a link
image

@jthorton jthorton linked an issue Oct 8, 2025 that may be closed by this pull request

@IAlibay IAlibay left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Couple of small things, otherwise lgtm!

Comment thread docs/concepts/estimators.rst
Comment thread docs/concepts/femap.rst Outdated
Comment thread docs/concepts/plotting.rst
Comment thread docs/concepts/plotting.rst Outdated
Absolute free energy plots compare calculated and experimental :math:`\Delta G` values for each ligand. Absolute
:math:`\Delta G` predictions can be obtained from a connected network of relative free energies. They therefore accumulate errors from multipule aspects of the calculation, including
edge accuracy, uncertainty quantification, perutration network design and estimator, giving a complete picture of protocol performance.
These plots are then useful for both method developers and users, as they show how well the overall ranking of ligands is predicted.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Same as above, this was resolved without action/comment, just checking if that was intended.

Comment thread docs/concepts/plotting.rst Outdated
@atravitz

atravitz commented Oct 8, 2025

Copy link
Copy Markdown
Collaborator

Looks like the theme has updated! Looks cool, but I don't think it is clear what is a link image

this will be resolved in the next ofe-sphinx-theme release with dark mode.

@atravitz

atravitz commented Oct 8, 2025

Copy link
Copy Markdown
Collaborator

@jthorton you can pin with git+https://github.com/OpenFreeEnergy/ofe-sphinx-theme@v0.2.0 if you'd like to decouple this PR from theme updates.

@IAlibay IAlibay left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

lgtm, thanks!

@IAlibay
IAlibay merged commit 9d02159 into main Oct 8, 2025
8 of 9 checks passed
@IAlibay
IAlibay deleted the core_concepts branch October 8, 2025 15:57
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.

Documentation: theory page

3 participants