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

E0 bug in multi-head fine tuning #371

Open
bernstei opened this issue Apr 6, 2024 · 3 comments
Open

E0 bug in multi-head fine tuning #371

bernstei opened this issue Apr 6, 2024 · 3 comments

Comments

@bernstei
Copy link
Collaborator

bernstei commented Apr 6, 2024

There's an E0-related bug in multi-head-finetune if you have more theories in the E0s.json file than you actually use (and maybe in related circumstances as well)

Initiaklly I ran with "pbe" and "r2scan" in the E0s json, and added my own "hse_d3" key list with its own E0 values. However, when I looked at the fitting log, I saw that the Atomic Energies was a list with 3 sub-lists, each with E0s for one theory, but the label was apparently stripped, since it was a list, not a dict. Then I noticed that the initial force and stress errors were identical to before, but the energy error was much larger. When I removed the r2scan and only has pbe and hse_d3 (in that order) E0s, everything made sense. I suspect the way the relevant E0s are extracted from the json dict is not quite right.

@bernstei
Copy link
Collaborator Author

In general, it should be possible to clean up the E0s, e.g. remove keys that aren't there in the list of Zs, make sure that any Z that is present had a value specified, and, for multihead, fill in something (0?) for any Z that is missing in one theory but present in the other.

@gabor1
Copy link
Collaborator

gabor1 commented Apr 22, 2024

Don't you think that putting in zeros for missing elements is bad? I'd rather not evaluate missing elements with that particular head at all! And if we don't evaluate, we shouldn't need to specify.

@bernstei
Copy link
Collaborator Author

bernstei commented Apr 22, 2024

Yes, we should make sure no Z from the data in a given head is missing a corresponding E0 entry. But the E0s dict is turned into a torch tensor and matrix-multiplied, so it has to have the same dimensions. I assume that the 0 entries then don't get used, but they need to be there.

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

No branches or pull requests

2 participants