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

WIP: Committee Model #234

Draft
wants to merge 7 commits into
base: main
Choose a base branch
from
Draft

Conversation

simonwengert
Copy link
Contributor

@simonwengert simonwengert commented Mar 16, 2023

This is an implementation of a Calculator-class for committee models. The Calculator-class is rather lightweight and makes use of a specific Committee-class (that stores the CommitteeMembers representing the committee model) which can be used to calibrate the obtained uncertainties (required when sub-sampling is used to create the training data of the committee members).

Some things to be discussed / not yet fully implemented:

  • Alter the Calculator to take a Committee-instance as input and, thus, separate Committee setup from its application
  • Read in calibrated parameters from disk (avoid recalibrating) + writing calibrated parameters to disk
  • Extend calibration for Atoms.arrays properties
  • Generalize scaling as sigma_scaled = alpha * sigma**(gamma/2 + 1) (with default gamma=0)
  • For CommitteeMembers allow different types for inputs filename and list(Atoms) and/or ConfigSet
  • Add tests
  • Add example for online documentation

@bernstei
Copy link
Contributor

bernstei commented Mar 16, 2023

Let's keep this consistent with https://github.com/ACEsuit/ACEHAL/, specifically ace_committee_model.py and bias_calc.py which uses it.

@bernstei
Copy link
Contributor

Also, is there a reason this is in workflow? I'd have thought it'd be OK as its own standalone module, which can get passed to generic.evaluate like any other calculator.

@simonwengert
Copy link
Contributor Author

As a tool to support the building of active learning workflows, there is an argument to have it directly in wfl, I would think.

@bernstei
Copy link
Contributor

As a tool to support the building of active learning workflows, there is an argument to have it directly in wfl, I would think.

Accessible from wfl, definitely, but why in? If there was a new DFT calculator we wanted to use for fitting a potential, we still wouldn't put it in wfl, just a wrapper that handles the wfl-specific functionality like running in separate directories.

@simonwengert
Copy link
Contributor Author

Accessible from wfl, definitely, but why in? If there was a new DFT calculator we wanted to use for fitting a potential, we still wouldn't put it in wfl, just a wrapper that handles the wfl-specific functionality like running in separate directories.

For a committee of ML models, I guess, I don't see as many ways to implement it as there are for DFT codes. There might be multiple approach, though, and we don't want to explicitly have all of them implemented. Migrating this to a separate module and in wfl itself focusing on the interface is fine for me, too.

@bernstei
Copy link
Contributor

I agree that there aren't as many ways to implement it, but I can definitely see it being used in many ways, possibly outside of workflow. I don't even really like my committee/bias calculators being inside ACEHAL, even though I put them there myself. Let's talk with others on the slack about where it belongs.

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