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

Add logic to check for incompatible parameters #935

Merged
merged 9 commits into from
Aug 21, 2024

Conversation

rvhonorato
Copy link
Member

@rvhonorato rvhonorato commented Jul 11, 2024

You are about to submit a new Pull Request. Before continuing make sure you read the contributing guidelines and that you comply with the following criteria:

  • You have sticked to Python. Please talk to us before adding other programming languages to HADDOCK3
  • Your PR is about CNS
  • Your code is well documented: proper docstrings and explanatory comments for those tricky parts
  • You structured the code into small functions as much as possible. You can use classes if there is a (state) purpose
  • Your code follows our coding style
  • You wrote tests for the new code
  • tox tests pass. Run tox command inside the repository folder
  • -test.cfg examples execute without errors. Inside examples/ run python run_tests.py -b
  • PR does not add any dependencies, unless permission granted by the HADDOCK team
  • PR does not break licensing
  • Your PR is about writing documentation for already existing code 🔥
  • Your PR is about writing tests for already existing code :godmode:

This PR adds a logic to check for parameter incompatibilities.

For example; with #918 I added a new parameter called less_io however you cannot use this parameter together with mode=batch.

So now you can define a new incompatible property of a given parameter in the haddock.modules.defaults.yaml;

less_io:
  # ...
  incompatible:
    mode: batch

Which will then be validated when the run is being setup.

@rvhonorato rvhonorato self-assigned this Jul 11, 2024
@rvhonorato rvhonorato linked an issue Jul 11, 2024 that may be closed by this pull request
@rvhonorato rvhonorato marked this pull request as ready for review July 11, 2024 14:15
@rvhonorato rvhonorato added execution Related to execution modes, such as GRID, HPC, local, etc. yaml default parameters Anything related to the YAML configuration files for default parameters labels Jul 11, 2024
amjjbonvin
amjjbonvin previously approved these changes Jul 11, 2024
src/haddock/modules/defaults.yaml Show resolved Hide resolved
VGPReys
VGPReys previously approved these changes Jul 12, 2024
sverhoeven added a commit to i-VRESSE/workflow-builder that referenced this pull request Jul 12, 2024
Manually edited catalog to test incompatible keyword

Refs haddocking/haddock3#935

TODO
- [ ] generate JSON schema
- [ ] handle incompatible fields being in a group
- [ ] handle multiple incompatible fields in same object, note allOf did not work
@sverhoeven
Copy link
Contributor

In clustrmsd module the value of criterion parameter determines which other module parameters are ignored see

long: Value of cutoff cophenetic distance. When criterion is maxclust, this value is ignored.
. Could we use incompatible parameters there as well? Or use clustrrmsd approach here?

@VGPReys
Copy link
Contributor

VGPReys commented Jul 12, 2024

In clustrmsd module the value of criterion parameter determines which other module parameters are ignored see

long: Value of cutoff cophenetic distance. When criterion is maxclust, this value is ignored.

. Could we use incompatible parameters there as well? Or use clustrrmsd approach here?

I don't know, because they have default values, and only in some case (based on criterion value) some parameters will be used but not the other ones.

@rvhonorato rvhonorato dismissed stale reviews from VGPReys and amjjbonvin via 411f865 July 12, 2024 13:38
amjjbonvin
amjjbonvin previously approved these changes Aug 5, 2024
Copy link
Member

@amjjbonvin amjjbonvin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We will have to remember to edit the default.yaml files when needed to define incompatible parameters, but now at least the machinery to check those is in place - nice!

VGPReys
VGPReys previously approved these changes Aug 5, 2024
@rvhonorato rvhonorato dismissed VGPReys’s stale review August 5, 2024 08:57

cannot add code and review at the same time

@rvhonorato rvhonorato merged commit 7a9434e into main Aug 21, 2024
4 checks passed
@rvhonorato rvhonorato deleted the 934-less-io-incompatible-with-batch-mode branch August 21, 2024 10:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
execution Related to execution modes, such as GRID, HPC, local, etc. yaml default parameters Anything related to the YAML configuration files for default parameters
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Less I/O incompatible with batch mode
5 participants