Skip to content

Issue #162 - scale box factor change #164

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.

Sign up for GitHub

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

Open
IAlibay wants to merge 4 commits into
base: main
Choose a base branch
from
Open

Issue #162 - scale box factor change #164

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
9855d75
This makes tests fail
IAlibay Nov 28, 2025
d1816b2
Merge branch 'main' into issue-162
IAlibay Dec 22, 2025
10f0c30
Fix things
IAlibay Dec 23, 2025
a5003d6
Add news item
IAlibay Dec 23, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 28 additions & 0 deletions news/issue-162.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
**Added:**

* <news item>

**Changed:**

* In line with Interchange v0.5+, the box is no longer being scaled by 1.1
when defining the ``number_of_solvent_molecules``. In those cases, the
resulting box density will be much higher.
To roughly reproduce previous behavior, we recommend reducing the
``target_density`` to 0.715 * unit.grams / unit.mL when
defining ``number_of_solvent_molecules``.
Comment on lines +10 to +12
Copy link
Collaborator

@j-wags j-wags Jan 22, 2026

Choose a reason for hiding this comment

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

(not blocking) Rewording with a bit more detail, if desired

Suggested change
To roughly reproduce previous behavior, we recommend reducing the
``target_density`` to 0.715 * unit.grams / unit.mL when
defining ``number_of_solvent_molecules``.
If you were calling PackmolSolvation before with a defined
``number_of_solvent_molecules`` and not providing
``target_density`` (default value 0.95 g/mL), then to recover
the same behavior you should manually set the target density
to 0.715 g/mL (0.95 x (1 / (1.1^3)). If you WERE manually setting
a ``number_of_solvent_molecules`` AND ``target density``,
then you should scale the ``target_density`` by 0.751 (1 / (1.1^3))
to recover the old behavior.


**Deprecated:**

* <news item>

**Removed:**

* <news item>

**Fixed:**

* <news item>

**Security:**

* <news item>
1 change: 1 addition & 0 deletions src/pontibus/tests/protocols/relative/test_protocol.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -586,6 +586,7 @@ def test_dry_run_complex_setnwaters(
solv_settings.solvation_settings = PackmolSolvationSettings(
number_of_solvent_molecules=15000,
solvent_padding=None,
target_density=0.715 * unit.grams / unit.mL,
)

protocol = HybridTopProtocol(settings=solv_settings)
Expand Down
10 changes: 8 additions & 2 deletions src/pontibus/tests/utils/test_system_creation.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -577,6 +577,7 @@ def test_nonwater_solvent_short(smc_components_benzene_named, smiles):
solvent_padding=None,
number_of_solvent_molecules=100,
assign_solvent_charges=True,
target_density=0.715 * unit.grams / unit.mL,
),
smc_components=smc_components_benzene_named,
protein_component=None,
Expand Down Expand Up @@ -1007,6 +1008,7 @@ def test_nonwater_solvent_long(solvent_smiles, solute_smiles):
solvent_padding=None,
number_of_solvent_molecules=1000,
assign_solvent_charges=True,
target_density=0.715 * unit.grams / unit.mL,
),
smc_components={ligand_smc: ligand_offmol},
protein_component=None,
Expand Down Expand Up @@ -1852,7 +1854,9 @@ def interchange_system(self, water_off, request):
],
),
solvation_settings=PackmolSolvationSettings(
number_of_solvent_molecules=500, solvent_padding=None
number_of_solvent_molecules=500,
solvent_padding=None,
target_density=0.715 * unit.grams / unit.mL,
),
smc_components=smc_components,
protein_component=protein_component,
Expand Down Expand Up @@ -1962,7 +1966,9 @@ def interchange_system(self, water_off, request, eg5_ligands, eg5_cofactor):
],
),
solvation_settings=InterchangeOpenMMSolvationSettings(
number_of_solvent_molecules=100, solvent_padding=None
number_of_solvent_molecules=100,
solvent_padding=None,
target_density=0.715 * unit.grams / unit.mL,
),
smc_components=smc_components,
protein_component=protein_component,
Expand Down
9 changes: 5 additions & 4 deletions src/pontibus/utils/settings.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -142,11 +142,12 @@ class BasePontibusSolvationSettings(BaseSolvationSettings):
target_density: GramsPerMolQuantity | None = 0.95 * unit.grams / unit.mL # noqa: F821
Copy link
Collaborator

@j-wags j-wags Jan 23, 2026

Choose a reason for hiding this comment

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

(blocking)

Suggested change
target_density: GramsPerMolQuantity | None = 0.95 * unit.grams / unit.mL # noqa: F821
target_density: GramsPerMolQuantity | None = 0.715 * unit.grams / unit.mL # noqa: F821

"""
Target mass density for the solvated system in units compatible with g / mL.
Generally a ``target_density`` value of 0.95 * unit.grams / unit.mL is
sufficient, although you may have to aim for a lower value should you find
it difficult to pack your system.
A ``target_density`` value of 0.95 * unit.grams / unit.mL works in many cases,
but may lead to an overly dense system. This is especially true when
defining ``number_of_solvent_molecules``, in this case a value of
0.715 * unit.grams / unit.mL is recommended.

Default: 0.95 * unit.grams / unit.mL.
Default: 0.715 * unit.grams / unit.mL.
Copy link
Member Author

@IAlibay IAlibay Jan 7, 2026

Choose a reason for hiding this comment

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

Should it be or should it not?
I.e. 50% of the times it should be one and 50% of the time it should not.

Copy link
Collaborator

@j-wags j-wags Jan 9, 2026

Choose a reason for hiding this comment

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

To ensure I understand before I misspeak/go down a rabbit hole - Could you clarify that the 50-50 comment means that

  • some code paths would need this default set to 0.715 to maintain the same default behavior given the box_scaleup_factor=1.0 change,
  • other code paths would maintain the same behavior without this being changed and would have different default behavior if this change is made

Copy link
Member Author

@IAlibay IAlibay Jan 16, 2026

Choose a reason for hiding this comment

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

Yes, when you define number_of_solvent_molecules which happens for non-water SFEs (e.g. MNSOL benchmarks), then you need to adjuste the target density to 0.715 unlike the previous 0.95.

However, when you don't (because you are instead just defining a solvent padding), which happens with water solvent SFEs (e.g. freesolv benchmarks), then the current 0.95 target density is correct and shouldn't be changed.

This matches the behaviour introduced in Interchange v0.5, where some code paths that didn't calculate the box density from the number of molecules (e.g. solvate_topology and solvate_topology_nonwater) wouldn't have been affected by the scale change and therefore wouldn't need their target density adjusted.


Notes
-----
Expand Down
4 changes: 2 additions & 2 deletions src/pontibus/utils/system_solvation.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,7 @@ def _box_density_from_mols(
"""
Approximate box size with known number and type of molecules.

Generate an approximate box size based on the number and molecular
Generate an approximate box size based on the number and molecular
weight of the molecules present, and a target density for the final
solvated mixture.

Expand Down Expand Up @@ -145,7 +145,7 @@ def _box_density_from_mols(
total_mass = molecules_total_mass + solute_total_mass
volume = total_mass / target_density

return _scale_box(box_shape, volume)
return _scale_box(box_shape, volume, box_scaleup_factor=1.0)


def _neutralize_and_pack_box(
Expand Down
Loading