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

Switch to pydantic models for MLIP hyperparameters and Config files ? #324

Open
naik-aakash opened this issue Jan 8, 2025 · 1 comment
Labels
enhancement New feature or request question Further information is requested

Comments

@naik-aakash
Copy link
Collaborator

naik-aakash commented Jan 8, 2025

Currently, we are relying on YAML files for default config files, which is working fine, but at this point, we do not have proper type checking in place for the parameters supplied by the users.

I was thinking that we can have better checks by simply defining pydantic models with defaults for parameters and configs. This way, we can easily have type checks for user-supplied parameters with minimal effort.

Another option would be to update the yaml files to specify expected datatype for each key value pair

@naik-aakash naik-aakash added enhancement New feature or request question Further information is requested labels Jan 8, 2025
@naik-aakash naik-aakash changed the title Switch to pydantic models for MLIP hyperparameters and Config files Switch to pydantic models for MLIP hyperparameters and Config files ? Jan 8, 2025
@dft-dutoit
Copy link
Collaborator

I think this is a great idea. In XPOT, we use json files for the user input, but only due to the different user journey. Failures relating to types occur within the setup of the first fitting, which results in 1) low cost for type errors, and 2) we allow the fitting method to error directly. In the case of autoplex, pydantic would be a great option for type checking.

Pydantic models would seem to me the better option than yaml files with expected datatypes, due to the greater flexibility for types, but I don't think that is strictly necessary here (perhaps when expanding further to other user inputs beyond currently implemented / used (hyper)parameters (e.g. urls). Tools exist for yaml conversion already, so implementation is definitely possible.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request question Further information is requested
Projects
None yet
Development

No branches or pull requests

2 participants