-
Notifications
You must be signed in to change notification settings - Fork 647
fix(pt-expt): preserve lower semantics in backend conversion #5975
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.
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
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -52,6 +52,18 @@ | |
| # --------------------------------------------------------------------------- | ||
| PT2_EXTRA_PREFIX = "model/extra/" | ||
|
|
||
| # Backend conversion supplies the source artifact's lower ABI. Each accepted | ||
| # source kind maps to the concrete schema emitted by pt_expt. PT SeZM's | ||
| # ``edge_vec`` lower and pt_expt's ``graph`` lower carry the same directed-edge | ||
| # model semantics, while the target materializes its native NeighborGraph ABI. | ||
| _LOWER_INPUT_KIND_TARGETS = { | ||
| "nlist": "nlist", | ||
| "graph": "graph", | ||
| "dpa1_canonical": "dpa1_canonical", | ||
| "dpa4c_canonical": "dpa4c_canonical", | ||
| "edge_vec": "graph", | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Blocking, though it should be a small fix. Mapping
Before this commit the same model converted fine: The new test only exercises an energy-fitting DPA4 config, so nothing catches it. The cleanest fix is probably to make the mapping conditional rather than a static table entry: resolve |
||
| } | ||
|
|
||
|
|
||
| def _strip_shape_assertions(graph_module: torch.nn.Module) -> None: | ||
| """Neutralise deferred shape-guard assertion nodes in an exported graph. | ||
|
|
@@ -1242,7 +1254,8 @@ def serialize_from_file(model_file: str) -> dict: | |
| dict | ||
| The serialized model data. If the archive contains | ||
| ``model_def_script.json`` (training config), it is included | ||
| under the ``"model_def_script"`` key. | ||
| under the ``"model_def_script"`` key. ``lower_input_kind`` records | ||
| the concrete lower ABI from the artifact metadata. | ||
| """ | ||
| if model_file.endswith(".pt2"): | ||
| return _serialize_from_file_pt2(model_file) | ||
|
|
@@ -1252,10 +1265,20 @@ def serialize_from_file(model_file: str) -> dict: | |
|
|
||
| def _serialize_from_file_pte(model_file: str) -> dict: | ||
| """Serialize a .pte model file to a dictionary.""" | ||
| extra_files = {"model.json": "", "model_def_script.json": ""} | ||
| extra_files = { | ||
| "model.json": "", | ||
| "model_def_script.json": "", | ||
| "metadata.json": "", | ||
| } | ||
| torch.export.load(model_file, extra_files=extra_files) | ||
| model_dict = json.loads(extra_files["model.json"]) | ||
| model_dict = _json_to_numpy(model_dict) | ||
| metadata = ( | ||
| json.loads(extra_files["metadata.json"]) if extra_files["metadata.json"] else {} | ||
| ) | ||
| model_dict["lower_input_kind"] = metadata.get( | ||
| "lower_input_kind", model_dict.get("lower_input_kind", "nlist") | ||
| ) | ||
| if extra_files["model_def_script.json"]: | ||
| model_dict["model_def_script"] = json.loads( | ||
| extra_files["model_def_script.json"] | ||
|
|
@@ -1273,6 +1296,7 @@ def _serialize_from_file_pt2(model_file: str) -> dict: | |
|
|
||
| model_json_entry = PT2_EXTRA_PREFIX + "model.json" | ||
| model_def_script_entry = PT2_EXTRA_PREFIX + "model_def_script.json" | ||
| metadata_entry = PT2_EXTRA_PREFIX + "metadata.json" | ||
| with zipfile.ZipFile(model_file, "r") as zf: | ||
| names = zf.namelist() | ||
| if model_json_entry not in names: | ||
|
|
@@ -1283,8 +1307,15 @@ def _serialize_from_file_pt2(model_file: str) -> dict: | |
| model_def_script_json = "" | ||
| if model_def_script_entry in names: | ||
| model_def_script_json = zf.read(model_def_script_entry).decode("utf-8") | ||
| metadata_json = "" | ||
| if metadata_entry in names: | ||
| metadata_json = zf.read(metadata_entry).decode("utf-8") | ||
| model_dict = json.loads(model_json) | ||
| model_dict = _json_to_numpy(model_dict) | ||
| metadata = json.loads(metadata_json) if metadata_json else {} | ||
| model_dict["lower_input_kind"] = metadata.get( | ||
| "lower_input_kind", model_dict.get("lower_input_kind", "nlist") | ||
| ) | ||
| if model_def_script_json: | ||
| model_dict["model_def_script"] = json.loads(model_def_script_json) | ||
| return model_dict | ||
|
|
@@ -1355,6 +1386,17 @@ def _resolve_lower_kind(model_file: str, data: dict, lower_kind: str) -> str: | |
| return "nlist" | ||
|
|
||
|
|
||
| def _resolve_target_lower_kind(model_file: str, data: dict, lower_kind: str) -> str: | ||
| """Resolve a source lower ABI to a concrete pt_expt export schema.""" | ||
| source_lower_kind = _resolve_lower_kind(model_file, data, lower_kind) | ||
| if source_lower_kind not in _LOWER_INPUT_KIND_TARGETS: | ||
| raise ValueError( | ||
| f"Unsupported lower_kind {source_lower_kind!r}; expected one of " | ||
| f"{sorted(_LOWER_INPUT_KIND_TARGETS)}." | ||
| ) | ||
| return _LOWER_INPUT_KIND_TARGETS[source_lower_kind] | ||
|
|
||
|
|
||
| def deserialize_to_file( | ||
| model_file: str, | ||
| data: dict, | ||
|
|
@@ -1393,14 +1435,17 @@ def deserialize_to_file( | |
| (``atype``/``n_node``/``edge_index``/``edge_vec``/``edge_mask`` and | ||
| the destination/source CSR views) with a DYNAMIC edge axis ``E`` | ||
| (``Dim("nedge", min=2)``), so the artifact accepts any system size. | ||
| ``"auto"`` (used by ``convert-backend``) resolves to ``"graph"`` for an | ||
| exportable graph-lower ``.pt2`` and ``"nlist"`` otherwise (see | ||
| :func:`_resolve_lower_kind`). A graph lower always preserves the fused | ||
| inference operators (``DP_CUDA_INFER >= 2``) and the per-atom virial. | ||
| ``"auto"`` resolves to ``"graph"`` for an exportable graph-lower | ||
| ``.pt2`` and ``"nlist"`` otherwise (see :func:`_resolve_lower_kind`). | ||
| Backend conversion passes the source artifact's concrete lower kind; | ||
| compatible source ABIs are mapped to the target's native schema while | ||
| preserving their execution semantics. A graph lower always preserves | ||
| the fused inference operators (``DP_CUDA_INFER >= 2``) and the | ||
| per-atom virial. | ||
| The selected schema is recorded as ``lower_input_kind`` in | ||
| ``metadata.json``. | ||
| """ | ||
| lower_kind = _resolve_lower_kind(model_file, data, lower_kind) | ||
| lower_kind = _resolve_target_lower_kind(model_file, data, lower_kind) | ||
| if data["model"].get("type") == "native_spin" and lower_kind not in ( | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Minor, but this guard can now be tripped by a value the caller never supplied, and its message then misdirects them.
The mismatch predates this PR, but |
||
| "graph", | ||
| "dpa4c_canonical", | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The ABI declaration is only half-rolled-out, so the result now depends on which file format the weights arrived in.
pt, tf, tf2 and jax all declare a kind, but
deepmd/dpmodel/utils/serialization.py(save_dp_model/load_dp_model) anddeepmd/pd/utils/serialization.pydo not, sodata.get(...)isNonefor a.dpsource and this sends"auto"._resolve_lower_kindthen picksgraph/dpa1_canonical/dpa4c_canonicalfor an eligible model — where the very same weights arriving as a.pthwould have been pinned to"nlist".So the same model converts to a different artifact depending on whether it came through
.dpor.pth, which is the class of surprise this PR is otherwise removing.doc/backend.mdframes the no-metadata path as a legacy/backward-compatibility fallback, but.dpis not legacy — it is a current format that will simply never carry the field unless it is added.Adding the declaration to the dpmodel serializer would close it. Paddle is less urgent, since its
serialize_from_fileraisesNotImplementedErrortoday, but it is the same gap.Related, lower stakes: the rejection just below fires for any target whose hook lacks a
lower_kindparameter, which includes.dp. Unlike.pb/.pth, a.dpbakes no execution schema at all — it is a weights container, and whoever re-exports it later picks their own lower. Blockingdp convert-backend sezm.pt out.dptherefore protects nothing while losing a working path. Worth narrowing the guard to targets that actually materialise a lower.