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

fix sign error in early_stopping.py #465

Merged
merged 1 commit into from
Feb 20, 2025

Conversation

ESEberhard
Copy link
Contributor

@ESEberhard ESEberhard commented Sep 26, 2024

I greatly appreciated your paper :)
While working with your code, I think I found a small but nevertheless consequential typo in your early stopping procedure,
than kept my runs from finishing.

@cw-tan
Copy link
Collaborator

cw-tan commented Oct 6, 2024

Hi @ESEberhard , thank you for spotting this and raising a PR! We have migrated towards using PyTorch Lightning (on the develop branch) and their callbacks, including their EarlyStopping callback here. I'm going to close this PR since we no longer depend on early_stopping.py -- sorry about that! Regardless, thank you very much for contributing.

@cw-tan cw-tan closed this Oct 6, 2024
@cw-tan cw-tan reopened this Feb 14, 2025
@cw-tan cw-tan force-pushed the fix-early-stopping branch from c6c9e5b to 66bdee2 Compare February 14, 2025 19:32
@Linux-cpp-lisp
Copy link
Collaborator

Thanks for catching this @ESEberhard !

@cw-tan
Copy link
Collaborator

cw-tan commented Feb 14, 2025

Thanks @ESEberhard, reopened to make sure that users using the pre-overhaul version of NequIP gets your fix. Will merge once the tests run successfully.

@cw-tan cw-tan mentioned this pull request Feb 15, 2025
@cw-tan cw-tan force-pushed the fix-early-stopping branch from 66bdee2 to 8de03ea Compare February 20, 2025 03:08
@cw-tan cw-tan merged commit 6a6110b into mir-group:main Feb 20, 2025
4 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.

3 participants