Skip to content

Commit cb1e44f

Browse files
committed
fix: align installation skill plan contracts
1 parent 90e4625 commit cb1e44f

6 files changed

Lines changed: 103 additions & 15 deletions

File tree

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

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -73,6 +73,17 @@ non-null.
7373

7474
Use the absolute manager and environment name from the plan. Install only the
7575
requested packages; do not add LAMMPS, Horovod, or MPI unless they are in scope.
76+
Treat `package.channels` as authoritative when it is non-empty and preserve its
77+
order by rendering one `-c` option per entry. This includes selected mirrors and
78+
pre-release channels such as `conda-forge/label/deepmd-kit_dev` or
79+
`conda-forge/label/deepmd-kit_rc`. Use `conda-forge` only when the list is empty.
80+
81+
```bash
82+
"<absolute-conda-or-mamba>" create -n "<environment-name>" \
83+
-c "<channel-1>" -c "<channel-2>" deepmd-kit
84+
```
85+
86+
For an empty `package.channels` list, render the stable default explicitly:
7687

7788
```bash
7889
"<absolute-conda-or-mamba>" create -n "<environment-name>" \

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

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -62,6 +62,12 @@ validate it, and render the gate again. A command containing an unassigned
6262
plan variable, an empty required argument, or `<placeholder>` is not
6363
executable.
6464

65+
For conda, compare every rendered `-c` argument with `package.channels`. An
66+
empty list uses only the stable `conda-forge` default; a non-empty list replaces
67+
that default and preserves the recorded order. For JAX C/C++, two null
68+
TensorFlow roots select the Python-library route. A single root selects its
69+
corresponding external library route; setting both is ambiguous and invalid.
70+
6571
## Wrong skill root
6672

6773
Helper scripts belong to the installed skill, not the DeePMD-kit checkout or

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

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -94,6 +94,9 @@ Use `null` for an inapplicable object. Do not add undeclared keys.
9494
- Keep the two DeePMD-kit index fields null for the default package index.
9595
Record a user-selected mirror or the documented pre-release index explicitly.
9696
- Keep `backend_index_url` null when the default package index is intended.
97+
- For conda, an empty `channels` list selects the stable `conda-forge`
98+
default. A non-empty list is authoritative and is rendered in order, with
99+
one `-c` option per channel. Keep this list empty for other methods.
97100
- Package indexes and download URLs use HTTPS.
98101
- Require exactly one of HTTPS `artifact_url` or absolute `artifact_path`, plus
99102
`sha256`, for `offline`.
@@ -149,8 +152,16 @@ entirely by its Python package. Require `cuda_home` for a CUDA build and
149152
Use a dedicated install prefix. Never choose `/`, a home directory, a conda
150153
prefix, `/usr`, `/usr/local`, or `$HOME/.local` as a disposable prefix.
151154

152-
For a JAX C++ backend, provide either `tensorflow_root` or
153-
`tensorflow_c_root`. For Paddle C++, provide `paddle_inference_dir`.
155+
For a JAX C++ backend, the TensorFlow dependency is selected by the two
156+
nullable roots:
157+
158+
- Keep both null to use the TensorFlow C++ libraries from the selected Python
159+
environment with `USE_TF_PYTHON_LIBS=ON`.
160+
- Set only `tensorflow_root` to use an external TensorFlow C++ installation.
161+
- Set only `tensorflow_c_root` to use the TensorFlow C library.
162+
163+
The two TensorFlow roots are mutually exclusive. For Paddle C++, provide
164+
`paddle_inference_dir`.
154165

155166
### `lammps`
156167

skills/deepmd-install/references/source-cpp.md

Lines changed: 15 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -35,15 +35,21 @@ build directory. Never delete or recursively replace the install prefix.
3535

3636
Start with all backends disabled, then enable the selected backend and its
3737
documented C API dependency. Enabling TensorFlow also enables the JAX C API in
38-
DeePMD-kit by design.
39-
40-
| Backend | Required CMake arguments |
41-
| ----------------------- | ----------------------------------------------------------------------------------- |
42-
| PyTorch | `-DENABLE_PYTORCH=ON -DUSE_PT_PYTHON_LIBS=ON` plus the PyTorch CMake prefix |
43-
| TensorFlow | `-DENABLE_TENSORFLOW=ON -DUSE_TF_PYTHON_LIBS=ON` and the selected Python executable |
44-
| JAX with TensorFlow C++ | `-DENABLE_TENSORFLOW=ON -DUSE_TF_PYTHON_LIBS=ON` |
45-
| JAX with TensorFlow C | `-DENABLE_JAX=ON -DCMAKE_PREFIX_PATH=<tensorflow-c-root>` |
46-
| Paddle | `-DENABLE_PADDLE=ON -DPADDLE_INFERENCE_DIR=<paddle-inference-dir>` |
38+
DeePMD-kit by design. For JAX, select exactly one route from the state of the
39+
nullable roots in the plan.
40+
41+
| Backend dependency | Plan state | Required CMake arguments |
42+
| ---------------------------------------- | ------------------------------- | ------------------------------------------------------------------------------------- |
43+
| PyTorch | Not applicable | `-DENABLE_PYTORCH=ON -DUSE_PT_PYTHON_LIBS=ON` plus the PyTorch CMake prefix |
44+
| TensorFlow | Not applicable | `-DENABLE_TENSORFLOW=ON -DUSE_TF_PYTHON_LIBS=ON` and the selected Python executable |
45+
| JAX with Python TensorFlow C++ libraries | Both TensorFlow roots are null | `-DENABLE_TENSORFLOW=ON -DUSE_TF_PYTHON_LIBS=ON` and the selected Python executable |
46+
| JAX with external TensorFlow C++ | Only `tensorflow_root` is set | `-DENABLE_TENSORFLOW=ON -DUSE_TF_PYTHON_LIBS=OFF -DTENSORFLOW_ROOT=<tensorflow-root>` |
47+
| JAX with TensorFlow C | Only `tensorflow_c_root` is set | `-DENABLE_JAX=ON -DCMAKE_PREFIX_PATH=<tensorflow-c-root>` |
48+
| Paddle | Not applicable | `-DENABLE_PADDLE=ON -DPADDLE_INFERENCE_DIR=<paddle-inference-dir>` |
49+
50+
The Python TensorFlow C++ route requires the planned interpreter to import a
51+
TensorFlow package that provides `libtensorflow_cc`. Record and install that
52+
package during the Python gate before configuring CMake.
4753

4854
For PyTorch, discover the prefix from the planned interpreter in the same shell
4955
call that configures CMake:

skills/deepmd-install/scripts/validate_plan.py

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -245,6 +245,9 @@ def _validate_package(
245245
for key in ("backend_packages", "channels"):
246246
if key in package:
247247
_validate_string_list(package[key], f"package.{key}", errors)
248+
channels = package.get("channels")
249+
if method != "conda" and isinstance(channels, list) and channels:
250+
errors.append("package.channels: non-empty channels require method 'conda'")
248251
for key in ("install_lammps", "install_ipi"):
249252
if key in package and not isinstance(package[key], bool):
250253
errors.append(f"package.{key}: expected a boolean")
@@ -376,6 +379,9 @@ def _validate_cpp(
376379
for key in ("install_prefix", "build_directory"):
377380
if not _is_absolute_path(cpp.get(key)):
378381
errors.append(f"cpp.{key}: expected an absolute path")
382+
for key in ("tensorflow_root", "tensorflow_c_root", "paddle_inference_dir"):
383+
if cpp.get(key) is not None and not _is_absolute_path(cpp.get(key)):
384+
errors.append(f"cpp.{key}: expected an absolute path or null")
379385
if not all(
380386
_is_absolute_path(cpp.get(key)) for key in ("install_prefix", "build_directory")
381387
) or not _is_absolute_path(source.get("directory")):
@@ -397,12 +403,11 @@ def _validate_cpp(
397403
forbidden.add(_canonical(environment_prefix))
398404
if prefix in forbidden:
399405
errors.append("cpp.install_prefix: select a dedicated, non-shared prefix")
400-
if backend == "jax" and not (
401-
_is_absolute_path(cpp.get("tensorflow_root"))
402-
or _is_absolute_path(cpp.get("tensorflow_c_root"))
406+
if backend == "jax" and all(
407+
cpp.get(key) is not None for key in ("tensorflow_root", "tensorflow_c_root")
403408
):
404409
errors.append(
405-
"cpp: JAX requires tensorflow_root or tensorflow_c_root for the C API"
410+
"cpp: JAX tensorflow_root and tensorflow_c_root are mutually exclusive"
406411
)
407412
if backend == "paddle" and not _is_absolute_path(cpp.get("paddle_inference_dir")):
408413
errors.append("cpp.paddle_inference_dir: Paddle C++ requires an absolute path")

source/tests/test_deepmd_install_skill.py

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -214,12 +214,51 @@ def test_validate_source_tensorflow_cpu_plan() -> None:
214214
assert PLAN.validate_plan(plan) == []
215215

216216

217+
def test_validate_jax_cpp_with_tensorflow_python_libraries() -> None:
218+
"""Allow JAX C++ to use TensorFlow libraries from the Python environment."""
219+
plan = _source_plan()
220+
plan["goal"] = "python+cpp"
221+
plan["backend"] = "jax"
222+
plan["lammps"] = None
223+
assert PLAN.validate_plan(plan) == []
224+
225+
226+
def test_validate_jax_cpp_rejects_ambiguous_tensorflow_roots() -> None:
227+
"""Require one unambiguous TensorFlow dependency route for JAX C++."""
228+
plan = _source_plan()
229+
plan["goal"] = "python+cpp"
230+
plan["backend"] = "jax"
231+
plan["lammps"] = None
232+
cpp = plan["cpp"]
233+
assert isinstance(cpp, dict)
234+
cpp["tensorflow_root"] = "/opt/tensorflow-cpp"
235+
cpp["tensorflow_c_root"] = "/opt/tensorflow-c"
236+
errors = PLAN.validate_plan(plan)
237+
assert any("mutually exclusive" in error for error in errors)
238+
239+
217240
@pytest.mark.parametrize("method", ["pip", "conda", "offline", "docker"])
218241
def test_validate_easy_install_plans(method: str) -> None:
219242
"""Accept the method-specific fields for each easy-install path."""
220243
assert PLAN.validate_plan(_easy_plan(method)) == []
221244

222245

246+
def test_validate_conda_channels_are_method_specific() -> None:
247+
"""Accept selected conda channels without ignoring them for other methods."""
248+
plan = _easy_plan("conda")
249+
package = plan["package"]
250+
assert isinstance(package, dict)
251+
package["channels"] = ["conda-forge/label/deepmd-kit_rc", "conda-forge"]
252+
assert PLAN.validate_plan(plan) == []
253+
254+
pip_plan = _easy_plan("pip")
255+
pip_package = pip_plan["package"]
256+
assert isinstance(pip_package, dict)
257+
pip_package["channels"] = ["conda-forge"]
258+
errors = PLAN.validate_plan(pip_plan)
259+
assert "package.channels: non-empty channels require method 'conda'" in errors
260+
261+
223262
def test_validate_offline_plan_requires_checksum() -> None:
224263
"""Reject an offline artifact without an integrity value."""
225264
plan = _easy_plan("offline")
@@ -841,6 +880,16 @@ def test_docker_reference_uses_backend_aware_verifier() -> None:
841880
assert "readonly" in reference
842881

843882

883+
def test_conda_reference_renders_planned_channels() -> None:
884+
"""Keep selected conda channels authoritative with a stable default."""
885+
reference = (
886+
REPOSITORY_ROOT / "skills" / "deepmd-install" / "references" / "easy-install.md"
887+
).read_text(encoding="utf-8")
888+
assert "package.channels" in reference
889+
assert '-c "<channel-1>" -c "<channel-2>"' in reference
890+
assert "Use `conda-forge` only when the list is empty" in reference
891+
892+
844893
def test_verify_lammps_dpa4c_cli(tmp_path: Path) -> None:
845894
"""Accept the exact DPA4C host and Kokkos pair styles."""
846895
binary = tmp_path / "lmp_dpa4c"

0 commit comments

Comments
 (0)