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] load user provided yamls using __post_init__ > avoids need to also have that file on remote server #307

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

naik-aakash
Copy link
Collaborator

@naik-aakash naik-aakash commented Dec 23, 2024

Closes #305

Changes

  1. Added __post_init__ method to RssMaker

Todo

  • providing default config file arg seems redundant (Maybe remove)
  • check if MLIPFitMaker also needs this change
  • update documentation (expose default hyperparameter file contents if not already done )
  • Add type checks before updating kwargs of config
  • add tests to ensure __post_init__ work as intended

@naik-aakash naik-aakash marked this pull request as draft December 23, 2024 06:26
@naik-aakash naik-aakash changed the title [WIP] load yamls using post_init > no need to copy to remote server [WIP] load user provided yamls using __post_init__ > avoids need to also have that file on remote server Dec 23, 2024
@naik-aakash naik-aakash added documentation Improvements or additions to documentation enhancement New feature or request labels Dec 23, 2024
@naik-aakash naik-aakash self-assigned this Dec 23, 2024
@JaGeo
Copy link
Collaborator

JaGeo commented Dec 23, 2024

Shouldn't we rather do something similar to atomate2 settings?

@JaGeo
Copy link
Collaborator

JaGeo commented Dec 23, 2024

@naik-aakash
Copy link
Collaborator Author

naik-aakash commented Dec 23, 2024

Hi @JaGeo , i think both above approach we can use to load the default yamls, which I plan to do next : similar to atomate2 settings

But we would still need postinit in the maker if we expect user to provide modified yaml path (so it gets read when the flow is created) and we don't want to copy that yamls to remote cluster. Due to nature of jobflow delayed execution

@JaGeo
Copy link
Collaborator

JaGeo commented Dec 23, 2024

@naik-aakash okay! Likely pymatgen approach is th closest to what we want for defaults

@JaGeo
Copy link
Collaborator

JaGeo commented Dec 23, 2024

Another thought: post init will not work if you initialize the flow during run time

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Better handling of user provided yaml files
2 participants