Skip to content

Commit 3cdeb63

Browse files
committed
fix: separate dp1s home from Python prefix
1 parent 6a9d4f6 commit 3cdeb63

5 files changed

Lines changed: 105 additions & 19 deletions

File tree

skills/deepmd-install/references/easy-install.md

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -117,7 +117,7 @@ to a shell:
117117

118118
```bash
119119
curl -fsSL https://dp1s.deepmodeling.com | env \
120-
DP1S_HOME="<absolute-environment-prefix>" \
120+
DP1S_HOME="<absolute-dp1s-home>" \
121121
DP1S_NO_PATH_UPDATE=1 \
122122
DEEPMD_VERSION="<deepmd-version>" \
123123
bash
@@ -126,8 +126,11 @@ curl -fsSL https://dp1s.deepmodeling.com | env \
126126
Omit the `DEEPMD_VERSION` assignment when `package.deepmd_version` is null.
127127
`DP1S_NO_PATH_UPDATE` prevents the installer from editing shell startup files.
128128
Apply only additional options selected from the official `dp1s` documentation.
129-
After installation, resolve the absolute interpreter from the installed `dp`
130-
entry point, record it in the plan, and re-run validation before verification.
129+
After installation, keep `environment.dp1s_home` unchanged. Resolve the absolute
130+
interpreter from the installed `dp` entry point, run that interpreter to obtain
131+
its `sys.prefix`, record both as `environment.python` and `environment.prefix`,
132+
and re-run validation before verification. The Python prefix normally differs
133+
from `dp1s_home`; pass only `environment.prefix` to `--expected-prefix`.
131134

132135
## Offline package
133136

skills/deepmd-install/references/failure-modes.md

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -68,10 +68,15 @@ argument with `package.channels`. An empty list uses only the stable
6868
recorded order. A non-null `package.deepmd_version` produces the exact
6969
`deepmd-kit=<version>` package constraint.
7070

71-
For `dp1s`, compare `DP1S_HOME` and the optional `DEEPMD_VERSION` with the plan,
72-
and require `DP1S_NO_PATH_UPDATE=1`. For JAX C/C++, two null TensorFlow roots
73-
select the Python-library route. A single root selects its corresponding
74-
external library route; setting both is ambiguous and invalid.
71+
For `dp1s`, compare `DP1S_HOME` with `environment.dp1s_home`, compare the
72+
optional `DEEPMD_VERSION` with the plan, and require
73+
`DP1S_NO_PATH_UPDATE=1`. Preserve `dp1s_home` after installation; record the
74+
installed interpreter's distinct `sys.prefix` as `environment.prefix` for the
75+
Python identity gate.
76+
77+
For JAX C/C++, two null TensorFlow roots select the Python-library route. A
78+
single root selects its corresponding external library route; setting both is
79+
ambiguous and invalid.
7580

7681
## Wrong skill root
7782

skills/deepmd-install/references/plan-schema.md

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,8 @@ Use `null` for an inapplicable object. Do not add undeclared keys.
5454
"python": "/absolute/path/to/python",
5555
"manager": null,
5656
"name": null,
57-
"prefix": "/absolute/environment/prefix"
57+
"prefix": "/absolute/environment/prefix",
58+
"dp1s_home": null
5859
}
5960
```
6061

@@ -63,9 +64,13 @@ Use `null` for an inapplicable object. Do not add undeclared keys.
6364
environment and before installing DeePMD-kit.
6465
- `manager` and `name`: required for `conda`; `manager` is the absolute
6566
`conda` or `mamba` executable.
66-
- `prefix`: required for `pip`, `source`, `venv`, `prefix`, `dp1s`, and offline
67+
- `prefix`: required for `pip`, `source`, `venv`, `prefix`, and offline
6768
installations. For conda, resolve and record both `sys.executable` and
6869
`sys.prefix` after creating the environment and before verification.
70+
- `dp1s_home`: required only for `dp1s`; it is the Pixi and exposed-binary root
71+
passed as `DP1S_HOME`, not the installed Python prefix. Before installation,
72+
keep `python` and `prefix` null. After installation, preserve `dp1s_home` and
73+
record the resolved interpreter and its `sys.prefix` together.
6974

7075
### `package`
7176

@@ -241,7 +246,8 @@ The validator enforces these invariants:
241246
"python": "/opt/conda/envs/deepmd/bin/python",
242247
"manager": null,
243248
"name": null,
244-
"prefix": "/opt/conda/envs/deepmd"
249+
"prefix": "/opt/conda/envs/deepmd",
250+
"dp1s_home": null
245251
},
246252
"package": {
247253
"deepmd_version": null,
@@ -289,7 +295,8 @@ before installing dependencies or building.
289295
"python": "/opt/conda/envs/deepmd/bin/python",
290296
"manager": null,
291297
"name": null,
292-
"prefix": "/opt/conda/envs/deepmd"
298+
"prefix": "/opt/conda/envs/deepmd",
299+
"dp1s_home": null
293300
},
294301
"package": {
295302
"deepmd_version": null,

skills/deepmd-install/scripts/validate_plan.py

Lines changed: 30 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,7 @@
4343
"smoke_test",
4444
}
4545
OBJECT_KEYS = {
46-
"environment": {"kind", "python", "manager", "name", "prefix"},
46+
"environment": {"kind", "python", "manager", "name", "prefix", "dp1s_home"},
4747
"package": {
4848
"deepmd_version",
4949
"deepmd_index_url",
@@ -234,12 +234,36 @@ def _validate_environment(
234234
errors.append(
235235
f"environment.{key}: conda requires an absolute path or null"
236236
)
237-
if kind in {"venv", "prefix"} and not _is_absolute_path(environment.get("prefix")):
238-
errors.append(f"environment.prefix: {kind} requires an absolute path")
239-
if method in {"dp1s", "offline"} and not _is_absolute_path(
240-
environment.get("prefix")
237+
if method == "dp1s":
238+
if not _is_absolute_path(environment.get("dp1s_home")):
239+
errors.append("environment.dp1s_home: dp1s requires an absolute path")
240+
python = environment.get("python")
241+
prefix = environment.get("prefix")
242+
unresolved = python is None and prefix is None
243+
resolved = _is_absolute_path(python) and _is_absolute_path(prefix)
244+
if not (unresolved or resolved):
245+
errors.append(
246+
"environment: dp1s python and prefix must both be null or absolute paths"
247+
)
248+
if (
249+
resolved
250+
and _is_absolute_path(environment.get("dp1s_home"))
251+
and _canonical(environment["dp1s_home"]) == _canonical(prefix)
252+
):
253+
errors.append(
254+
"environment: dp1s_home and resolved Python prefix must be distinct"
255+
)
256+
elif environment.get("dp1s_home") is not None:
257+
errors.append("environment.dp1s_home: allowed only for method 'dp1s'")
258+
dp1s_prefix_pending = method == "dp1s" and environment.get("prefix") is None
259+
if (
260+
kind in {"venv", "prefix"}
261+
and not dp1s_prefix_pending
262+
and not _is_absolute_path(environment.get("prefix"))
241263
):
242-
errors.append(f"environment.prefix: {method} requires an absolute path")
264+
errors.append(f"environment.prefix: {kind} requires an absolute path")
265+
if method == "offline" and not _is_absolute_path(environment.get("prefix")):
266+
errors.append("environment.prefix: offline requires an absolute path")
243267
if method == "docker" and kind != "container":
244268
errors.append("environment.kind: docker requires 'container'")
245269

source/tests/test_deepmd_install_skill.py

Lines changed: 49 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,7 @@ def _source_plan() -> dict[str, object]:
5555
"manager": None,
5656
"name": None,
5757
"prefix": "/opt/deepmd",
58+
"dp1s_home": None,
5859
},
5960
"package": {
6061
"deepmd_version": None,
@@ -117,6 +118,7 @@ def _easy_plan(method: str) -> dict[str, object]:
117118
"manager": None,
118119
"name": None,
119120
"prefix": "/opt/deepmd",
121+
"dp1s_home": None,
120122
}
121123
package: dict[str, object] = {
122124
"deepmd_version": None,
@@ -137,6 +139,15 @@ def _easy_plan(method: str) -> dict[str, object]:
137139
environment.update(
138140
{"kind": "conda", "python": None, "manager": "/opt/conda", "name": "deepmd"}
139141
)
142+
elif method == "dp1s":
143+
environment.update(
144+
{
145+
"kind": "prefix",
146+
"python": None,
147+
"prefix": None,
148+
"dp1s_home": "/opt/dp1s",
149+
}
150+
)
140151
elif method == "offline":
141152
environment.update({"kind": "prefix", "python": None})
142153
package.update(
@@ -237,7 +248,7 @@ def test_validate_jax_cpp_rejects_ambiguous_tensorflow_roots() -> None:
237248
assert any("mutually exclusive" in error for error in errors)
238249

239250

240-
@pytest.mark.parametrize("method", ["pip", "conda", "offline", "docker"])
251+
@pytest.mark.parametrize("method", ["pip", "conda", "dp1s", "offline", "docker"])
241252
def test_validate_easy_install_plans(method: str) -> None:
242253
"""Accept the method-specific fields for each easy-install path."""
243254
assert PLAN.validate_plan(_easy_plan(method)) == []
@@ -272,6 +283,41 @@ def test_validate_conda_channels_are_method_specific() -> None:
272283
assert "package.channels: non-empty channels require method 'conda'" in errors
273284

274285

286+
def test_validate_dp1s_lifecycle_keeps_home_and_prefix_distinct(tmp_path: Path) -> None:
287+
"""Accept unresolved and resolved dp1s identities without conflating paths."""
288+
plan = _easy_plan("dp1s")
289+
environment = plan["environment"]
290+
assert isinstance(environment, dict)
291+
dp1s_home = tmp_path / "dp1s-home"
292+
python_prefix = dp1s_home / "envs" / "dp1s"
293+
environment["dp1s_home"] = str(dp1s_home)
294+
assert PLAN.validate_plan(plan) == []
295+
296+
environment["python"] = str(python_prefix / "bin" / "python")
297+
environment["prefix"] = str(python_prefix)
298+
assert environment["dp1s_home"] != environment["prefix"]
299+
assert PLAN.validate_plan(plan) == []
300+
301+
environment["prefix"] = environment["dp1s_home"]
302+
errors = PLAN.validate_plan(plan)
303+
assert any("must be distinct" in error for error in errors)
304+
305+
environment["prefix"] = str(python_prefix)
306+
environment["python"] = None
307+
errors = PLAN.validate_plan(plan)
308+
assert any("must both be null or absolute paths" in error for error in errors)
309+
310+
311+
def test_validate_dp1s_home_is_method_specific() -> None:
312+
"""Reject a dp1s installer root that another method would ignore."""
313+
plan = _easy_plan("pip")
314+
environment = plan["environment"]
315+
assert isinstance(environment, dict)
316+
environment["dp1s_home"] = "/opt/dp1s"
317+
errors = PLAN.validate_plan(plan)
318+
assert "environment.dp1s_home: allowed only for method 'dp1s'" in errors
319+
320+
275321
@pytest.mark.parametrize("field", ["python", "prefix"])
276322
def test_validate_conda_resolved_paths_must_be_absolute(field: str) -> None:
277323
"""Reject a relative Conda interpreter or prefix after environment creation."""
@@ -934,9 +980,10 @@ def test_dp1s_reference_preserves_planned_identity() -> None:
934980
reference = (
935981
REPOSITORY_ROOT / "skills" / "deepmd-install" / "references" / "easy-install.md"
936982
).read_text(encoding="utf-8")
937-
assert 'DP1S_HOME="<absolute-environment-prefix>"' in reference
983+
assert 'DP1S_HOME="<absolute-dp1s-home>"' in reference
938984
assert 'DEEPMD_VERSION="<deepmd-version>"' in reference
939985
assert "DP1S_NO_PATH_UPDATE=1" in reference
986+
assert '--expected-prefix "<absolute-environment-prefix>"' in reference
940987

941988

942989
def test_verify_lammps_dpa4c_cli(tmp_path: Path) -> None:

0 commit comments

Comments
 (0)