Migrate validation to Protocol._validate in HybridTopologyProtocol #1740
Conversation
IAlibay
changed the title
IAlibay
changed the title
|
pre-commit.ci autofix |
for more information, see https://pre-commit.ci
| if m.componentA not in alchemical_components["stateA"]: | ||
| raise ValueError( | ||
| f"Mapping componentA {m.componentA} not in alchemical components of stateA" | ||
| ) | ||
| if m.componentB not in alchemical_components["stateB"]: | ||
| raise ValueError( | ||
| f"Mapping componentB {m.componentB} not in alchemical components of stateB" | ||
| ) |
There was a problem hiding this comment.
Could maybe squash the repeated errors into a single loop?
| if m.componentA not in alchemical_components["stateA"]: | |
| raise ValueError( | |
| f"Mapping componentA {m.componentA} not in alchemical components of stateA" | |
| ) | |
| if m.componentB not in alchemical_components["stateB"]: | |
| raise ValueError( | |
| f"Mapping componentB {m.componentB} not in alchemical components of stateB" | |
| ) | |
| for label in ["A", "B"]: | |
| if getattr(m, f"component{label}") not in alchemical_components[f"state{label}"]: | |
| raise ValueError(f"Mapping component{label} {gatattr(m, f'component{label}') not in alchemical components of state{label}") | |
There was a problem hiding this comment.
Thanks, I implemented a variant of this. I'm generally not a massive fan of using getattr, but I guess if it can be reduced to the one call then it's ok.
| * Mappings which involve element changes in core atoms | ||
| """ | ||
| # if a single mapping is provided, convert to list | ||
| if isinstance(mapping, ComponentMapping): |
There was a problem hiding this comment.
This is done for us in the default_validate method on the base class so we could skip it.
There was a problem hiding this comment.
Sorry I might be missing something - it's not doing it inplace / in self, so we have to do it again here?
hannahbaumann
left a comment
hannahbaumann
left a comment
There was a problem hiding this comment.
Thanks @IAlibay , love the validate method! Just some small spelling things.
|
|
||
| # Validate charge difference | ||
| # Note: validation depends on the mapping & solvent component checks | ||
| if stateA.contains(SolventComponent): |
There was a problem hiding this comment.
More a note to self, we'll have to see how to best do all these validations with the explicit solvent component.
There was a problem hiding this comment.
I think this is where we will need to make an explicit solvent component be a subclasses of both protein and solvent components
Co-authored-by: Hannah Baumann <[email protected]> Co-authored-by: Josh Horton <[email protected]>
|
No API break detected ✅ |
Fixes #1739
Checklist
newsentry, or the changes are not user-facing.pre-commit.ci autofixbefore requesting review.Manual Tests: these are slow so don't need to be run every commit, only before merging and when relevant changes are made (generally at reviewer-discretion).
Developers certificate of origin