lab-cosmo / metatrain

Training and evaluating machine learning models for atomistic systems.

https://lab-cosmo.github.io/metatrain/

BSD 3-Clause "New" or "Revised" License

13 stars 3 forks source link

Suggestions for closely matching allowed names #271

Closed PicoCentauri closed 6 days ago

PicoCentauri commented 1 week ago

Merge after #241 !

As suggested by @Luthaf in https://github.com/lab-cosmo/metatrain/pull/241#discussion_r1647332552 this PR adds suggestions for unknown fields when passing options.

I am using the default cutoff in get_close_matches() so that we don't display anything if the unknown name is to "far" away. For example when you face-roll on your keyboard you should not get a suggestion.

Contributor (creator of pull-request) checklist

[x] Tests updated (for new features and bugfixes)?
[x] Documentation updated (for new features)?
[x] Issue referenced (for PRs that solve an issue)?

📚 Documentation preview 📚: https://metatrain--271.org.readthedocs.build/en/271/

frostedoyster commented 1 week ago

I'm finding it difficult to review because it's also showing all the changes from the other PR. I've tried changing the base branch but it isn't working... let's wait until #241 is merged

PicoCentauri commented 6 days ago

In pet it is possible to indicate not only "atomic" batch size, but also normal "structural" batch size. The same applies to SCHEDULER_STEP_SIZE, EPOCH_NUM, and EPOCHS_WARMUP. See more details in https://github.com/lab-cosmo/metatrain/blob/528f232e372c0103ba5031828ead30d5a94827ce/docs/src/architectures/pet.rst. Thus, the possibility for a user to indicate structural versions should be preserved.

You mean that one can either use SCHEDULER_STEP_SIZE or SCHEDULER_STEP_SIZE_ATOMIC but not both?

frostedoyster commented 6 days ago

@PicoCentauri I opened an issue regarding the comment by @spozdn: https://github.com/lab-cosmo/metatrain/issues/277