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

Generate catalog with tomlschema #50

Merged
merged 15 commits into from
Apr 5, 2022
Merged

Conversation

sverhoeven
Copy link
Member

@sverhoeven sverhoeven commented Mar 9, 2022

Refs #47

The catalogs have been regenerated with newer haddock3 yaml files.
The diff shows both the application of tomlschema to fold mol_1 and mol_2 into mol and documented/remove/added/moved parameters,

@netlify
Copy link

netlify bot commented Mar 9, 2022

Deploy Preview for wonderful-noether-53a9e8 ready!

Name Link
🔨 Latest commit 8e7aba8
🔍 Latest deploy log https://app.netlify.com/sites/wonderful-noether-53a9e8/deploys/624c2c0f23fef100088011d6
😎 Deploy Preview https://deploy-preview-50--wonderful-noether-53a9e8.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

@sverhoeven sverhoeven mentioned this pull request Mar 14, 2022
@sverhoeven sverhoeven marked this pull request as ready for review March 16, 2022 11:56
@sverhoeven sverhoeven requested a review from Peter9192 March 22, 2022 14:43
@sverhoeven
Copy link
Member Author

sverhoeven commented Mar 24, 2022

Noticed that hisd and hise parameter in topoaa node in easy catalog are written as hisd = [123] instead of hisd_1 = 123. So still some work todo.

@Peter9192
Copy link
Contributor

Tried to run the code but didn't manage to install haddock correctly. The instructions about linking the CNS executable are unclear. How am I supposed to obtain this executable?

I also wondered whether you are planning to write some small tests for this script.

@Peter9192
Copy link
Contributor

Found it: http://cns-online.org/v1.3/, continuing

@Peter9192
Copy link
Contributor

Please print out this page with your Institution's letterhead, sign and email a scanned PDF to [email protected] .

Okay I'm gonna call it a day 😞

@sverhoeven
Copy link
Member Author

hisd and hise are now indexed as well.

description: Defines a molecule as a shape.
$comment: Defines a molecule as a shape, which is a collection of beads.
type: boolean
mol_fix_origin:
Copy link
Contributor

Choose a reason for hiding this comment

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

Should this category have a title, too?

When selecting expert > rigidbody > molecule, the parameter 'mol_fix_origin' appears, and lacks a descriptive title.

Copy link
Contributor

@Peter9192 Peter9192 Mar 30, 2022

Choose a reason for hiding this comment

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

The name seems to come from here: https://github.com/haddocking/haddock3/blob/e324b9cbd4c9c76ea1df6ce874e8ba5317486218/src/haddock/modules/sampling/rigidbody/defaults.yaml#L4

Would it make sense to inherit title and description from the array items, or are they not always the same for all items?

Copy link
Member Author

Choose a reason for hiding this comment

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

Good of you to notice.

I need to get those titles/descriptions from haddock3 so I created haddocking/haddock3#370

Copy link
Contributor

@Peter9192 Peter9192 left a comment

Choose a reason for hiding this comment

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

It seems to work well. @bvreede and I had a good time trying to understand all the regexes and how you exclude all options starting from the most complex ones to the simpler ones.

There's a few quirks that I guess are hard to iron out completely, and given that there's no tests I assume you don't want to make a very big effort to make this script perfect. So please go ahead and merge after looking at our few small questions :-)

array_of_array_of_scalar = r'(\w+)_1_1'
array_of_array_of_object = r'(\w+)_(\w+)_1_1'
skip = r'\w+?(_\d\d?)(_\d\d?)?'
must_be_array_of_scalar = {'mol_fix_origin_1', 'mol_shape_1'}
Copy link
Contributor

Choose a reason for hiding this comment

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

it seems this is hardcoded because the regex can't distinguish between

  • mol_shape_1 -> mol_shape:[1]
  • mol_shape_1 -> mol:[{shape}]

So basically any key that consists of multiple name segments separated by underscores is confusing. Is it discouraged to create new names like that in Haddock? Should there be a warning if a new parameter like this is encountered?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yep, parameters starting with mol need get special treatment. See https://github.com/haddocking/haddock3/blob/main/src/haddock/gear/expandable_parameters.py#L2-L72 how haddock3 code treats these parameters.

if 'group' in v:
# Move group from nested prop to outer array
new_config[p]['group'] = v['group']
del v['group']
Copy link
Contributor

Choose a reason for hiding this comment

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

why must the group be deleted?

Copy link
Contributor

Choose a reason for hiding this comment

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

So as a side-effect of calling this function, the incoming config is modified?

Copy link
Member Author

Choose a reason for hiding this comment

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

The array will be in that group and the scalar value is a child of that array. So the scalar is already in the group, by keeping a group on the scalar it would be rendered in its own collapsible subform.

You would then have a collapsible subforms inside an array itemwhich was already inside a collapsible subform felt a bit to much folding going on.

Also the grouper currently only works on parameters that are not nested.

util/generate_haddock3_catalog.py Outdated Show resolved Hide resolved
util/generate_haddock3_catalog.py Outdated Show resolved Hide resolved
import sys
from yaml import dump, load, Loader

from haddock.modules import modules_category
from haddock import config_expert_levels
from haddock import config_expert_levels, _hidden_level
Copy link
Contributor

Choose a reason for hiding this comment

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

Not sure what _hidden_level is

Copy link
Member Author

Choose a reason for hiding this comment

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

Hidden level is the level beyond guru and used for experimental or very obscure parameters

def process_global(level):
package = 'haddock.modules'
module = importlib.import_module(package)
with open(module.modules_defaults_path) as f:
optional_global_parameters = load(f, Loader=Loader)
config = REQUIRED_GLOBAL_PARAMETERS | optional_global_parameters
gmodule = importlib.import_module('haddock.gear.parameters')
Copy link
Contributor

Choose a reason for hiding this comment

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

much cleaner 👍

Copy link
Member Author

Choose a reason for hiding this comment

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

thanks

Copy link
Member Author

@sverhoeven sverhoeven left a comment

Choose a reason for hiding this comment

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

Thanks for reviewing, I know these first reviews can be hard to do.

import sys
from yaml import dump, load, Loader

from haddock.modules import modules_category
from haddock import config_expert_levels
from haddock import config_expert_levels, _hidden_level
Copy link
Member Author

Choose a reason for hiding this comment

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

Hidden level is the level beyond guru and used for experimental or very obscure parameters

def process_global(level):
package = 'haddock.modules'
module = importlib.import_module(package)
with open(module.modules_defaults_path) as f:
optional_global_parameters = load(f, Loader=Loader)
config = REQUIRED_GLOBAL_PARAMETERS | optional_global_parameters
gmodule = importlib.import_module('haddock.gear.parameters')
Copy link
Member Author

Choose a reason for hiding this comment

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

thanks

@sverhoeven sverhoeven merged commit 067fb67 into main Apr 5, 2022
@sverhoeven sverhoeven deleted the generate-catalog-with-tomlschema branch April 5, 2022 11:47
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.

3 participants