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

make topoaa.molX and mol_ parameter expandable #350

Merged
merged 11 commits into from
Mar 4, 2022
Merged

make topoaa.molX and mol_ parameter expandable #350

merged 11 commits into from
Mar 4, 2022

Conversation

joaomcteixeira
Copy link
Member

@joaomcteixeira joaomcteixeira commented Mar 2, 2022

  • Your code is well documented. Functions and classes have proper docstrings as explained in contributing guidelines. You wrote explanatory comments for those tricky parts.
  • 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
  • You have stick to Python. Talk with us before if you really need to add other programming languages to HADDOCK3
  • code follows our coding style
  • You avoided structuring the code into classes as much as possible, using small functions instead. But, you can use classes if there's a purpose.
  • PR does not add any install dependencies unless permission was granted by the HADDOCK team
  • PR does not break licensing
  • You get extra bonus if you write tests for already existing code :godmode:

Define mol parameters as expandable parameters. These include the molX in topoaa and mol_fix_origin and mol_shape in some other modules.

For topoaa the prot_segid is defined by the alphabet progression unless the user has defined a value.

todo:

  • implement forced progression for params except mol_

@joaomcteixeira joaomcteixeira changed the title Molexp make topoaa.molX and mol_ parameter expandable. Mar 2, 2022
@joaomcteixeira joaomcteixeira changed the title make topoaa.molX and mol_ parameter expandable. make topoaa.molX and mol_ parameter expandable Mar 2, 2022
@codecov-commenter
Copy link

codecov-commenter commented Mar 2, 2022

Codecov Report

Merging #350 (ebd9191) into main (528d7b6) will increase coverage by 1.19%.
The diff coverage is 93.91%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #350      +/-   ##
==========================================
+ Coverage   59.49%   60.69%   +1.19%     
==========================================
  Files          65       65              
  Lines        4022     4152     +130     
==========================================
+ Hits         2393     2520     +127     
- Misses       1629     1632       +3     
Impacted Files Coverage Δ
src/haddock/libs/libutil.py 64.10% <ø> (ø)
src/haddock/gear/prepare_run.py 48.71% <80.85%> (+9.34%) ⬆️
src/haddock/core/defaults.py 85.71% <100.00%> (+1.09%) ⬆️
src/haddock/gear/expandable_parameters.py 100.00% <100.00%> (ø)
tests/test_gear_expandable_parameters.py 100.00% <100.00%> (ø)
tests/test_gear_prepare_run.py 100.00% <100.00%> (ø)
tests/test_module_topoaa.py 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 528d7b6...ebd9191. Read the comment docs.

@joaomcteixeira joaomcteixeira marked this pull request as ready for review March 2, 2022 21:11
@joaomcteixeira
Copy link
Member Author

Define mol parameters as expandable parameters.

These include the molX in topoaa and mol_fix_origin and mol_shape in some other modules.

For topoaa the prot_segid is defined by the alphabet progression unless the user has defined a value.

capri.tsv values are the same as in main for all the test.cfg cases.

@joaomcteixeira joaomcteixeira self-assigned this Mar 2, 2022
@joaomcteixeira joaomcteixeira added feature New feature request m|emref emref module m|flexref flexref module m|mdref mdref module m|topoaa topoaa module m|rigidbody rigidbody sampling labels Mar 2, 2022
@joaomcteixeira joaomcteixeira added this to the v3.0.0 stable release milestone Mar 2, 2022
@joaomcteixeira joaomcteixeira linked an issue Mar 2, 2022 that may be closed by this pull request
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.

How does it work? Will it create by default all values (e.g. mol_fix_origin and mol_shape), set those to the default values and write them to the CNS inp files? And of course take any value the user might define in the config file?

Also how will that connect to the i-vresse workflow builder/editor?

Also note that there are other molecule-related parameters that could be handled that way, e.g. nseg_1 nfle_1 nrair_1 and all associated parameters.

@joaomcteixeira
Copy link
Member Author

How does it work? Will it create by default all values (e.g. mol_fix_origin and mol_shape), set those to the default values and write them to the CNS inp files? And of course take any value the user might define in the config file?

Yes, but only for the number of molecules input. For example, if the user provides two molecules and defines mol_fix_origin_2 = True. Then, a mol_fix_origin_1 = False will be written also to the CNS inp. But, mol_fix_origin_3 won't be written because only two molecules are in the input.

Also, if the user gives only two molecules and writes mol_shape_4 by mistake, HADDOCK3 will raise an error when reading the config file.

Also how will that connect to the i-vresse workflow builder/editor?

We need to investigate that. i-vresse could read from the python functions identifying which parameters are expandable, and create the appropriate menus for those.

Also note that there are other molecule-related parameters that could be handled that way, e.g. nseg_1 nfle_1 nrair_1 and all associated parameters.

Yes, but those are different. Those are associated with the rair_sta_1_1 parameters, for example. nseg_1 (and others) are a different kind of parameters that we can decide to populate automatically from the number of groups defined in the rair_sta_1_1 type of parameters. The rair_sta_1_1 type of parameter are expandable, as we have seen before.

@rvhonorato
Copy link
Member

Approved but I don't understand anything about this part, trusting you @joaomcteixeira :)

@joaomcteixeira
Copy link
Member Author

I am making a presentation for tomorrow. I think it's best to explain to all

@amjjbonvin
Copy link
Member

amjjbonvin commented Mar 3, 2022 via email

@joaomcteixeira
Copy link
Member Author

These are molecule specific as the first index relates to the molecule and the second to the segment.

Yes. One thing I can do is to block to the maximum number of molecules also.

@joaomcteixeira
Copy link
Member Author

Okay, I will add that in another issue, because this one was about cleaning the implementation and adding the mol

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.

Looking good - one last comment:

Shouldn't the yaml file for the topoaa module be update so that the documentation (short and long) explains that this block and parameters can be duplicated for additional molecules?

@joaomcteixeira
Copy link
Member Author

Added the documentation.
FYI, in the python shell, I treat these special cases of nested dictionaries with having or not the default key. That is, actual parameters have the default key, and if default is not present, the entry is treated as a subdictionary, as is the case of mol1. Need not to worry about this, was just to let it here stated. 👍
If you agree it can be merged.

@joaomcteixeira joaomcteixeira added this to In progress in Expandable parameters Mar 4, 2022
@amjjbonvin amjjbonvin merged commit 1ac124a into main Mar 4, 2022
Expandable parameters automation moved this from In progress to Done Mar 4, 2022
@amjjbonvin amjjbonvin deleted the molexp branch March 4, 2022 13:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature New feature request m|emref emref module m|flexref flexref module m|mdref mdref module m|rigidbody rigidbody sampling m|topoaa topoaa module
Development

Successfully merging this pull request may close these issues.

Make mol* parameters in topoaa also expandable
4 participants