improve ligand network description #1172
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #1172 +/- ##
==========================================
- Coverage 94.63% 92.47% -2.17%
==========================================
Files 143 143
Lines 10962 10962
==========================================
- Hits 10374 10137 -237
- Misses 588 825 +237
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
this is my first pass, very open to feedback @IAlibay |
IAlibay
left a comment
IAlibay
left a comment
There was a problem hiding this comment.
Couple of comments, otherwise looks good to me.
| connecting all :class:`.SmallMoleculeComponent` with a ''maximal network'' (using :func:`.generate_maximal_network`), | ||
| but it is much more efficient to use a network with less transformations like a ''radial network'' (also known as a star map, using :func:`.generate_radial_network`) | ||
| or a ''minimimal spanning network'' (using :func:`.generate_minimal_spanning_network`). | ||
| A :class:`.LigandNetwork` is a network where nodes are :class:`.SmallMoleculeComponent`\ s and edges are :class:`.LigandAtomMapping`\ s. |
There was a problem hiding this comment.
What's possibly missing here is some kind of exposition that tells users that edges are not always expected. I.e. you can have a ligand networks with disconnected sections.
|
|
||
| Any :class:`.LigandNetwork`` generation can be generally conceptualized into three steps: | ||
| Because a ``LigandNetwork``'s edges are atom mappings, each edge can be assigned a score that indicates the mapping's quality. | ||
| Some network generators use these scores to construct more efficient network topologies. |
There was a problem hiding this comment.
I'm not sure I fully understand this section, it feels a little bit "out of place". I'm probably missing a step, but in my head I'm thinking "ligand networks are generated using scores" not "there are scores and they are used to generate new networks".
There was a problem hiding this comment.
Not every network generator requires a scorer. For example, radial network doesn't require a scorer at all. I'm working on writing this up in a clearer way.
There was a problem hiding this comment.
I think there is a miscommunication here and the current text still has the same issue. The way things are such that the edges can have scores and networks can use these scores, but you already have a network, so the network isn't using these scores at that point. The way it should have been written imho is "the network may have been generated using these scores".
|
I think the current state is an improvement, but I intend to do more work on it as a part of the konnektor usability drive. OpenFreeEnergy/konnektor#152 |
|
No API break detected ✅ |
resolves #1017
Checklist
newsentryDevelopers certificate of origin