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

Better handling of prior configuration #280

Merged
merged 3 commits into from
Feb 14, 2024

Conversation

RaulPPelaez
Copy link
Collaborator

@RaulPPelaez RaulPPelaez commented Feb 13, 2024

I realized there is no way to set prior arguments from the yaml file. Also, the model can handle more than one prior, but there was no way to set more than one from the yaml file.
I updated the documentation to reflect this, which was wrong about it.

With this PR one can do something like:

prior_model:
  Atomref:
    max_z: 100
  Coulomb:
      alpha: 1
      max_num_neighbors: 10
      distance_scale: 1

Allow to configure more than one prior in the yaml file.
@RaulPPelaez
Copy link
Collaborator Author

@stefdoerr please review, this will come in handy for #279 too.

@RaulPPelaez
Copy link
Collaborator Author

I realized there is a much simpler change that can be done to instead allow this:

prior_model:
  Atomref:
    max_z: 100
  Coulomb:
      alpha: 1
      max_num_neighbors: 10
      distance_scale: 1

@peastman
Copy link
Collaborator

We already support setting args for multiple priors with exactly this syntax. It was added in #134. See in particular #134 (comment).

@RaulPPelaez
Copy link
Collaborator Author

Thanks for the ref!. It was undocumented and the argparser was preventing it from being used from the CLI.

@stefdoerr
Copy link
Collaborator

By the way I would not bother too much with CLI arguments. I think passing a yaml file is the more usual use case, parsing CLI is a pain

@stefdoerr stefdoerr merged commit 417f8a0 into torchmd:main Feb 14, 2024
2 checks passed
@RaulPPelaez RaulPPelaez deleted the prior_input branch February 14, 2024 11:33
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.

None yet

3 participants