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

Add combination of bonds primitive and trust radius optimiser #345

Merged
merged 41 commits into from
Jul 20, 2024

Conversation

shoubhikraj
Copy link
Collaborator

@shoubhikraj shoubhikraj commented Jul 10, 2024

(1) Adds a linear combination of bonds as a primitive
(2) Trust radius quasi-Newton optimiser


Checklist

  • The changes include an associated explanation of how/why
  • Test pass
  • Documentation has been updated
  • Changelog has been updated

Copy link

codecov bot commented Jul 10, 2024

Codecov Report

Attention: Patch coverage is 98.45758% with 6 lines in your changes missing coverage. Please review.

Project coverage is 97.43%. Comparing base (76398fc) to head (fb4328b).

Files Patch % Lines
autode/opt/optimisers/qa.py 92.53% 5 Missing ⚠️
autode/opt/optimisers/crfo.py 98.91% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           v1.4.4     #345      +/-   ##
==========================================
+ Coverage   97.39%   97.43%   +0.04%     
==========================================
  Files         202      204       +2     
  Lines       23278    23626     +348     
==========================================
+ Hits        22672    23021     +349     
+ Misses        606      605       -1     
Flag Coverage Δ
unittests 97.43% <98.45%> (+0.04%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

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

@shoubhikraj shoubhikraj changed the base branch from master to v1.4.4 July 10, 2024 20:49
@shoubhikraj shoubhikraj marked this pull request as ready for review July 11, 2024 08:30
autode/opt/coordinates/base.py Outdated Show resolved Hide resolved
autode/opt/coordinates/base.py Outdated Show resolved Hide resolved
autode/opt/coordinates/primitives.py Outdated Show resolved Hide resolved
autode/opt/coordinates/primitives.py Outdated Show resolved Hide resolved
autode/opt/optimisers/crfo.py Outdated Show resolved Hide resolved
autode/opt/optimisers/crfo.py Outdated Show resolved Hide resolved
autode/opt/optimisers/crfo.py Outdated Show resolved Hide resolved
autode/opt/optimisers/qa.py Show resolved Hide resolved
Copy link
Member

@t-young31 t-young31 left a comment

Choose a reason for hiding this comment

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

Looks great – left some small suggestions 🚀

autode/opt/optimisers/qa.py Outdated Show resolved Hide resolved
autode/opt/optimisers/qa.py Outdated Show resolved Hide resolved
tests/test_opt/test_crfo.py Show resolved Hide resolved
tests/test_opt/test_qa.py Show resolved Hide resolved
@shoubhikraj
Copy link
Collaborator Author

@t-young31 Hi, do you know why the tests are failing with a RecursionError on a numpy allclose call? I have no idea how this error is happening.

@t-young31
Copy link
Member

t-young31 commented Jul 14, 2024

looks like a major numpy update (https://github.com/duartegroup/autodE/actions/runs/9922667636/job/27421474592?pr=345#step:5:148) – will have a look for a solution.


edit: Looks like we're hitting:

https://github.com/numpy/numpy/blob/7bff2754f64cb855c39b59cf6d0b7bc51fe49a61/numpy/_core/numeric.py#L2419

and np.allclose(self, other)

♻️

@shoubhikraj
Copy link
Collaborator Author

@t-young31 Could you have one last check before merge please?

Copy link
Member

@t-young31 t-young31 left a comment

Choose a reason for hiding this comment

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

🚀

@shoubhikraj shoubhikraj merged commit eb3d475 into duartegroup:v1.4.4 Jul 20, 2024
14 checks passed
@shoubhikraj shoubhikraj deleted the driving-coords branch July 20, 2024 11:42
@shoubhikraj shoubhikraj restored the driving-coords branch July 20, 2024 18:45
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.

2 participants