Skip to content
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
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
5 changes: 5 additions & 0 deletions tyff/_tests/compute/test_prep.py
Original file line number Diff line number Diff line change
Expand Up @@ -87,4 +87,9 @@ def test_pressure_in_gas_configs(dhvap_entry):

for this_config in compute_configs:
for val in this_config:
# both liquid and gas configs should have a pressure key
assert "pressure" in val

# liquid configs must have non-null pressure, gas configs must have None
assert (val["tag"] == "liquid") == (val["pressure"] is not None)
assert (val["tag"] == "gas") == (val["pressure"] is None)
11 changes: 7 additions & 4 deletions tyff/compute/_equilibrate.py
Original file line number Diff line number Diff line change
Expand Up @@ -92,13 +92,14 @@ def _run_equilibration(
if topology.getPeriodicBoxVectors() is not None:
simulation.context.setPeriodicBoxVectors(*topology.getPeriodicBoxVectors())

# let pressure be None for gas-phase NVT simulations, including reporting as None to TensorReporter later
pressure = compute_config.get("pressure", None)
Comment on lines +95 to 96

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Fixed by guarding the reporter pressure before the gas-phase TensorReporter call: pressure is converted to None when absent and is only multiplied by kPa inside the liquid-only path. The liquid PressureNotDefinedError guard remains in place. Commit: 1b9a02a.


if pressure is None:
raise PressureNotDefinedError("Trying to set up NPT simulation but no pressure defined.")

# only set pressure in liquid NPT simulations, not in gas-phase NVT simulations
if compute_config["tag"] == "liquid":
if pressure is None:
raise PressureNotDefinedError("Trying to set up NPT simulation in liquid phase but no pressure defined.")

barostat = openmm.MonteCarloBarostat(
(pressure * openmm.unit.kilopascal).value_in_unit(openmm.unit.bar), # pressure in bar
compute_config["temperature"], # temperature in kelvin
Expand Down Expand Up @@ -136,12 +137,14 @@ def _run_equilibration(
compute_config["replicate_index"] + 1,
)

pressure_for_report = None if pressure is None else pressure * openmm.unit.kilopascal

simulation.reporters.append(
tyff.mm.TensorReporter(
output_file=files["msgpack_trajectory"].filepath,
report_interval=1000,
beta=1.0 / (openmm.unit.MOLAR_GAS_CONSTANT_R * compute_config["temperature"] * openmm.unit.kelvin),
pressure=pressure * openmm.unit.kilopascal,
pressure=pressure_for_report,
)
)

Expand Down
16 changes: 10 additions & 6 deletions tyff/compute/_produce.py
Original file line number Diff line number Diff line change
Expand Up @@ -91,19 +91,21 @@ def _run_production(
logger.info("Reinitializing context (in production step)")
simulation.context.reinitialize(preserveState=True)

# let pressure be None for gas-phase NVT simulations, including reporting as None to TensorReporter later
pressure = compute_config.get("pressure", None)
Comment on lines +94 to +95

if compute_config["tag"] == "liquid":
if pressure is None:
# this clause should already be triggered in _run_equilibration
raise PressureNotDefinedError("Trying to set up NPT simulation but no pressure defined.")

# just double check pressure is loaded back in correctly from state
barostat = next(
force for force in simulation.system.getForces() if isinstance(force, openmm.MonteCarloBarostat)
)

assert barostat.getDefaultPressure() is not None

pressure = compute_config.get("pressure", None)

if pressure is None:
raise PressureNotDefinedError("Trying to set up NPT simulation but no pressure defined.")

simulation.context.setVelocitiesToTemperature(
compute_config["temperature"], # kelvin, but as float
compute_config["replicate_index"] + 1,
Expand Down Expand Up @@ -131,12 +133,14 @@ def _run_production(
)
)

pressure_for_report = None if pressure is None else pressure * openmm.unit.kilopascal

simulation.reporters.append(
tyff.mm.TensorReporter(
output_file=files["msgpack_trajectory"].filepath,
report_interval=1000,
beta=1.0 / (openmm.unit.MOLAR_GAS_CONSTANT_R * compute_config["temperature"] * openmm.unit.kelvin),
pressure=pressure * openmm.unit.kilopascal,
pressure=pressure_for_report,
)
)

Expand Down
2 changes: 1 addition & 1 deletion tyff/compute/prep.py
Original file line number Diff line number Diff line change
Expand Up @@ -164,7 +164,7 @@ def _make_enthalpy_of_vaporization_compute_configs(
smiles=data_entry["smiles"],
x=data_entry["x"],
temperature=data_entry["temperature"],
pressure=data_entry["pressure"],
pressure=None, # should *always* be None, but not validated in the model
),
)
)
Expand Down
2 changes: 1 addition & 1 deletion tyff/configs/_compute.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,4 +18,4 @@ class BaseComputeConfig(typing_extensions.TypedDict, extra_items=typing.Any):

temperature: float

pressure: float
pressure: float | None
8 changes: 8 additions & 0 deletions tyff/configs/gas.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,14 @@


class VacuumGas(BaseComputeConfig):
"""
General model for vacuum simulations

Note that `pressure` is inherited from the base class as `float | None`,
but should always be `None` in this model.

"""

tag: typing.Literal["gas"]

"""
Expand Down