Add benchmark for model evaluation on small clusters#547
Conversation
ElliottKasoar
left a comment
There was a problem hiding this comment.
Hi @frostedoyster, thanks for this, it's looking really nice already, and should be a really interesting addition!
@joehart2001 and I will try to look in more detail as soon as we can, but I've left a few comments/questions from an initial pass of everything.
|
|
||
| Input/reference data: | ||
|
|
||
| * Cluster structures and reference forces are distributed as a separate zip archive and |
There was a problem hiding this comment.
Can you add comments on how the structures were originally obtained/built, and the level of theory of the reference data?
There was a problem hiding this comment.
i agree, it would be useful to have a lot of info from your email
|
Overall its looking great! Would we want to apply dispersion corrections for models which have not been trained on dispersion corrected data? We have the utility to do this automatically, see here. Im not suggesting that this needs dispersion, just would like to check |
|
Thanks for the comments and sorry for the delay! I've tried to address all the comments, let me know what you think.
My understanding is that meta-GGA and hybrid functionals (which we use here) are generally able to capture short- and medium-range dispersion effects, therefore I would suspect that keeping the tests as they are is probably the safest approach. Long-range dispersion effects are not captured, but these clusters are all short-ranged. My concern is that, if we removed dispersion artificially from the predictions of models that were trained with dispersion, we would probably overcorrect and make the models look artificially worse on this specific test. That said, I'm not an expert, so I'm curious to hear your thoughts |
Yes i definitely agree for r2scan, and we never artificially remove the dispersion dont worry. But i was wondering for |
|
Hi @frostedoyster, thanks for all the changes, it's looking great! Please can you take a look at our new filtering guidelines: https://ddmms.github.io/ml-peg/developer_guide/filter.html, and make any changes accordingly? The principles are relatively simple, but you do have to be a little careful, so if anything is unclear, please do ask! (You may need to rebase first) Another small note, but if you can run the pre-commit to make all of the automated checks pass, that would be great. |
|
Hey @joehart2001 and @ElliottKasoar, thanks for the review! I've updated some of the code to handle the new conventions (rebasing, element filtering, app changes). Regarding the comparison between PBE and hybrid with or without dispersion corrections, I think this is a hard problem in general. My instinct would be to leave PBE models as is, but I leave it up to you guys to decide, especially because I suspect that this decision would impact other benchmarks in the suite too. |
|
Hi @frostedoyster i think its looking and working pretty great. just some minor comments from me. @ElliottKasoar could you take a look? |
|
i would argue that if we should have the option to see with ans without dispersion correction as this would be the most informative. |
Pre-review checklist for PR author
PR author must check the checkboxes below when creating the PR.
Summary
As described in #546, this benchmark evaluates force accuracies on small atomic clusters.
Linked issue
Resolves #546
Progress
Potential aspects of the benchmark to be discussed with the maintainers:
Testing
We carefully checked consistency of the labels with the publicly available OMol25 and MAD-1.5 datasets. The benchmark has not been tested on any models yet.
New decorators/callbacks
No new callbacks are needed.