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

Move config tolerances to utils #66

Closed
wants to merge 1 commit into from

Conversation

JosePizarro3
Copy link
Collaborator

Hi @ladinesa

I moved some of the config.normalize values to here, as the normalization will now be contained in the datamodel. For now, I only added the relevant config values found in default.yaml in NOMAD, but this has to be extended when needed in normalization.

What do you think?

Closes #65

Fix model_system.py and spectral_profile modules with the new config tolerances
@JosePizarro3 JosePizarro3 requested a review from ladinesa May 13, 2024 13:19
@JosePizarro3 JosePizarro3 self-assigned this May 13, 2024
@JosePizarro3 JosePizarro3 linked an issue May 13, 2024 that may be closed by this pull request
@coveralls
Copy link

Pull Request Test Coverage Report for Build 9063623048

Details

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 98.22%

Totals Coverage Status
Change from base Build 8896824175: 0.0%
Covered Lines: 607
Relevant Lines: 618

💛 - Coveralls

@ladinesa
Copy link
Collaborator

imho, these are indeed schema-specific settings however, there should be a mechanism to override these values though the nomad config file. My only concern if you move it at this time is that these are also used in the legacy schema, hence you will have to update in two places. So my advice is to keep them in nomad for the mean time.

@JosePizarro3
Copy link
Collaborator Author

imho, these are indeed schema-specific settings however, there should be a mechanism to override these values though the nomad config file. My only concern if you move it at this time is that these are also used in the legacy schema, hence you will have to update in two places. So my advice is to keep them in nomad for the mean time.

Maybe I can move the default values to the nomad.yaml of the repo, but I am not sure how to load them then for the main nomad installation. Perhaps config_tolerances.py should be moved to its own subfolder and have some functionality to initialize the default values from the nomad.yaml?

About the legacy schema, all fine. It can stay with its own config for now. We can think on different names.

@ladinesa
Copy link
Collaborator

imho, these are indeed schema-specific settings however, there should be a mechanism to override these values though the nomad config file. My only concern if you move it at this time is that these are also used in the legacy schema, hence you will have to update in two places. So my advice is to keep them in nomad for the mean time.

Maybe I can move the default values to the nomad.yaml of the repo, but I am not sure how to load them then for the main nomad installation. Perhaps config_tolerances.py should be moved to its own subfolder and have some functionality to initialize the default values from the nomad.yaml?

they should be in defaults.yaml I think. I am not sure about schemas but we should probably do something similar to parsers where the kwargs are loaded when module is loaded.

About the legacy schema, all fine. It can stay with its own config for now. We can think on different names.

@JosePizarro3
Copy link
Collaborator Author

The problem is if someone wants to use this plugin in an OASI how do they set up these parameters? In my head, I always imagine that people will touch anyways the functionalities and define their tolerances in Python, so they will hardly use a yaml to pass the configs of these parameters. But I'd might be wrong, you tell me.

@ladinesa
Copy link
Collaborator

The problem is if someone wants to use this plugin in an OASI how do they set up these parameters? In my head, I always imagine that people will touch anyways the functionalities and define their tolerances in Python, so they will hardly use a yaml to pass the configs of these parameters. But I'd might be wrong, you tell me.

The plugin has defaults, and nomad has defaults.yaml or nomad.yaml which overrides these if defined.

@JosePizarro3
Copy link
Collaborator Author

The problem is if someone wants to use this plugin in an OASI how do they set up these parameters? In my head, I always imagine that people will touch anyways the functionalities and define their tolerances in Python, so they will hardly use a yaml to pass the configs of these parameters. But I'd might be wrong, you tell me.

The plugin has defaults, and nomad has defaults.yaml or nomad.yaml which overrides these if defined.

Ok, not sure how these defaults are defined. Do you mean in the nomad_plugin.yaml? Can I define a new key and use it for these defaults?

@JosePizarro3
Copy link
Collaborator Author

Closing this in favor of #64

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.

Move config tolerances to utils
3 participants