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 Hessian to C-api #1071

Merged
merged 1 commit into from
Sep 9, 2024
Merged

Add Hessian to C-api #1071

merged 1 commit into from
Sep 9, 2024

Conversation

TyBalduf
Copy link
Contributor

@TyBalduf TyBalduf commented Jul 30, 2024

Adds an API interface to the calculator type's hessian method. Unlike the underlying method, the parameters (e.g. step size and list of atoms to include) and other return values (dipole gradient) are all optional.

@TyBalduf
Copy link
Contributor Author

TyBalduf commented Aug 8, 2024

The Mac cpx failure seems unrelated, as all the changes here should only affect the C-API.

@TyBalduf
Copy link
Contributor Author

@awvwgk sorry to ping you on this pull request as well, but I wasn't sure who else would be comfortable reviewing changes to the C-API.

@marcelmbn
Copy link
Member

Can you update your feature branch with the latest updates from grimme-lab:main and check if the CI failure is still present?

@TyBalduf
Copy link
Contributor Author

TyBalduf commented Sep 5, 2024

@marcelmbn just pulled the latest from main and it looks like everything is passing now.

marcelmbn
marcelmbn previously approved these changes Sep 9, 2024
Copy link
Member

@marcelmbn marcelmbn left a comment

Choose a reason for hiding this comment

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

LGTM.

@marcelmbn
Copy link
Member

After merging the first of your C-API changes, there seems to be a conflict. Please resolve, so that we can merge this PR as well.

Signed-off-by: Ty Balduf <ty.balduf@schrodinger.com>
@TyBalduf
Copy link
Contributor Author

TyBalduf commented Sep 9, 2024

Was able to resolve the conflict, just needed to bump the API version and make sure that different test cases were affecting each others reference values. Should be good to go now.

@marcelmbn marcelmbn merged commit d42779f into grimme-lab:main Sep 9, 2024
13 checks passed
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