This repository was archived by the owner on Mar 10, 2026. It is now read-only.
Conversation
…ceptor parameters
17 tasks
Contributor
There was a problem hiding this comment.
Pull Request Overview
This PR corrects critical issues in hydrogen bond energy calculation and parameter handling in the forcefield implementation. It standardizes the hydrogen bond function signature to the canonical donor-hydrogen-acceptor order and fixes the well_depth values in DREIDING forcefield parameters to be positive values as expected by the potential function.
- Updated hydrogen bond calculation to use proper donor-hydrogen-acceptor ordering
- Fixed hydrogen bond parameter files to use positive well_depth values
- Enhanced parameterization logic with improved donor/acceptor detection based on forcefield types
Reviewed Changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| data/forcefield/*.toml | Changed well_depth values from negative to positive in hydrogen bond parameters |
| crates/scream-core/src/core/forcefield/energy.rs | Updated calculate_hbond function signature to donor-hydrogen-acceptor order |
| crates/scream-core/src/core/forcefield/scoring.rs | Fixed hydrogen bond key format and function call order |
| crates/scream-core/src/core/forcefield/params.rs | Added donor/acceptor sets and parsing logic for hydrogen bond parameters |
| crates/scream-core/src/core/forcefield/parameterization.rs | Enhanced parameterization with improved hydrogen bond role detection |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary:
Corrects a critical issue in the hydrogen bond energy calculation and refines the overall parameterization logic for increased accuracy and robustness. The hydrogen bond function signature was updated to the canonical
donor-hydrogen-acceptororder, and theScorernow correctly identifies donor/acceptor pairs based on pre-computed sets in theForcefieldparameters. Additionally, this fix includes corrections to the H-bond parameter files, ensuring thatwell_depthis always a positive value as expected by the potential function.Changes:
Corrected Hydrogen Bond Calculation Logic:
EnergyCalculator::calculate_hbondfunction signature to the standard(donor, hydrogen, acceptor)order for clarity and correctness.Scorerto correctly identify hydrogen bond donor/acceptor pairs based on forcefield type and connectivity.Forcefieldparameter loader now pre-computes sets of H-bond donor and acceptor atom types for efficient lookup during scoring.Fixed H-Bond Forcefield Parameters:
well_depthvalues in alldreidingforcefield TOML files to be positive, aligning them with the physical definition of a potential well depth.Refined Parameterization and Atom Roles:
Parameterizernow uses a more robust two-pass process: first assigning atom roles (Backbone,Sidechain, etc.) across the entire system, and then applying physicochemical parameters.