Repository navigation
Conversation
frlai
left a comment
There was a problem hiding this comment.
This is great. I previously had wanted to solve this problem but my solution would have been worse than this one.
There was a problem hiding this comment.
call this traced int. we should add a new folder to datatypes called primitives and put this there.
There was a problem hiding this comment.
Moved to datatypes/primitives/traced_int.py as TracedInt, exported from leapp.leapp_graph.datatypes.
| import torch | ||
|
|
||
|
|
||
| class TracedInt(int): |
There was a problem hiding this comment.
can this also be a subclass of traced_data at the same time? this will make it so leapp sees it as a tracable data and will propagate the traced state.
An isolated prototype confirmed that this works and allows existing structure traversal, eager unwrapping, proxy extraction, and context discovery to recognize traced sizes. That would reduce separate scalar-specific traversal and validation logic.
as a subclass of traced data this would probably also need to define things properties like _tensor and _data that help return the right data type. I can handle enabling this to become a valid input or output of a leapp node.
There was a problem hiding this comment.
Done: TracedInt now subclasses int and TracedData (with tensor/data views), and the scalar-specific find/proxy/owner helpers are gone in favour of the generic traversal. Node I/O is left for you, as you suggested.
| other integers records the operation so derived sizes stay symbolic. | ||
| """ | ||
|
|
||
| def __new__(cls, value: int, proxy, context, owner): |
There was a problem hiding this comment.
copy.copy() and copy.deepcopy() currently fail because reconstruction calls TracedInt.__new__ without its tracing arguments. This can probably be fixed with explicit copy hooks.
another probe when i was checking this behavior revealed that if someone tries to serialize this value for example save it in a yaml, that might fail. the agent suggested using __reduce_ex__ returning (int, (int(self),)), avoiding serialization of the live tracer.
There was a problem hiding this comment.
Added __copy__/__deepcopy__ (same value) and __reduce_ex__ returning (int, (value,)). yaml.safe_dump still rejected the subclass, so I also registered an int representer; both YAML dumpers now write a plain integer. Covered in tests/unit_tests/test_traced_int.py.
| return self._record(operator.mod, self, other) if isinstance(other, int) else NotImplemented | ||
|
|
||
| def __neg__(self): | ||
| return self._record(operator.sub, 0, self) |
There was a problem hiding this comment.
_record() currently creates proxies unconditionally. Please check the owning context’s current tracing status before recording arithmetic, validate operands from other contexts, and handle proxies belonging to a replaced graph. Arithmetic after capture finishes should execute normally without appending operations to an inactive graph.
There was a problem hiding this comment.
Arithmetic now records only while the owning node is tracing on its current graph and otherwise returns a plain int. Operands from another active node raise the usual mixed-context error, and stale operands (finished node or replaced graph) are used as constants.
|
note to self: Scalar-aware consumers — if adopting audit consumers that assume every |
There was a problem hiding this comment.
additionally this feature probably warrants unit tests.
There was a problem hiding this comment.
Added tests/unit_tests/test_traced_int.py (19 tests): traversal, every arithmetic op, copy/pickle/YAML, after capture, after graph replacement, stale and cross-context operands, narrow with a scalar bound, and assignment replay.
|
can you target develop instead of main |
torch.nonzero, boolean masks, masked_select and torch.unique return
tensors whose size depends on input values. Reading such a size
(x.shape[0], x.size(0), x.numel()) returned a plain int, so the traced
count was baked into the exported graph. A later input with a
different count then failed in ONNX Runtime, e.g. "cannot reshape {3}
to {2}". The TorchScript exporter keeps these sizes dynamic.
Tensors produced by those ops, and tensors computed from them, now
answer size queries with TracedInt: an int that carries the FX proxy
of the size query. Integer arithmetic on it is recorded, it is passed
to ops as its proxy, and torch.tensor(x.shape) is recorded as
scalar_tensor/stack so the exporter traces it. Tensors with static
shapes still return plain ints, so their graphs are unchanged.
len() always returns a plain int in Python, so it cannot stay
symbolic; the debugging guide says to use x.size(0) instead.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: Lionel Gulich <lgulich@nvidia.com>
Address review feedback on the data-dependent shape support: - Move TracedInt to leapp/leapp_graph/datatypes/primitives and export it from leapp.leapp_graph.datatypes. - TracedInt now subclasses both int and TracedData, so the generic structure traversal, unwrapping, proxy extraction and context validation recognize it. This replaces the scalar-specific find/proxy helpers and the owning-tensor back reference. - Arithmetic records only while the owning node is tracing and its graph is current; otherwise it returns a plain int. Operands from another active node are rejected, and operands from a finished or replaced graph are used as constants. The same applies to traced integers passed to tensor ops, indexing and torch.tensor(). - Consumers that assumed every TracedData is a tensor now handle traced integers: narrow() bounds use the integer proxy directly instead of .item(), an assigned traced integer becomes scalar_tensor(), and torch.tensor()/np.asarray() of a finished traced integer stay plain. - copy.copy/deepcopy return the same value, pickling stores a plain int, and YAML dumps it as an integer. - Add unit tests for TracedInt. Signed-off-by: Lionel Gulich <lgulich@nvidia.com> Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
f57c3eb to
191be0f
Compare
Drop TracedInt.__format__, which int.__format__ already provides, and the redundant traced-integer check before _graph_sizes(), which performs the same check itself. Signed-off-by: Lionel Gulich <lgulich@nvidia.com> Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Rebased onto On your audit note: |
Problem
torch.nonzero, boolean masks (x[x > 0]),masked_selectandtorch.uniquereturn tensors whose size depends on the input values. Reading such a size during tracing (x.shape[0],x.size(0),x.numel()) returned a plain Pythonint, so the traced count was baked into the exported graph:compile_graph(validate=True)then fails as soon as two samples produce different counts:Input shape:{3,1}, requested shape:{2,1}. If all samples happen to give the same count, validation passes and the bundle fails at deployment instead. The TorchScript exporter (torch.onnx.export(..., dynamo=False)) keeps the same code dynamic.We hit this while exporting a ProtoMotions humanoid policy (Metatron) through LEAPP.
Fix
TracedInt(leapp/leapp_graph/datatypes/primitives/traced_int.py). It subclasses bothintandTracedDataand carries the FX proxy of thesize/numelcall, so LEAPP's generic traversal, unwrapping, proxy extraction and context checks treat it like other traced data:+ - * // %) is recorded, but only while its node is tracing on the current graph. After capture, or for a replaced graph, it behaves as a plainint. Operands from another active node raise the usual mixed-context error;torch.Size, shape tuples, and getitem/setitem keys.narrowbounds use the integer proxy directly;copy/deepcopyreturn the same value, it pickles as a plainint(__reduce_ex__), and YAML dumps it as an integer.torch.tensor/as_tensorpatch recordstorch.tensor(x.shape)asscalar_tensor+stack. The TorchScript tracer keepstorch.tensor(traced ints)constant but traces those two.len(x)can't be made symbolic, because CPython coerces__len__to an exactint; the guide says to usex.size(0)instead.TracedIntas a node input or output.Testing
tests/functional_tests/test_data_dependent_shapes.pycoversreshapewith anonzerocount,size/numelwith integer arithmetic, boolean-mask counts,torch.tensor(x.shape), and static shapes staying plain ints. Each test exports with two samples of different counts, validates, and checksInferenceManageragainst eager on an unseen third count. All five fail without this change.tests/unit_tests/test_traced_int.py(19 tests) covers theTracedDataviews and traversal, every arithmetic op and the recorded target, copy/pickle/YAML, behaviour after capture and after graph replacement, stale and cross-context operands,narrowwith a scalar bound, and replay of slice and value assignments.develop: 1079 passed (develop: 1055). The same 9 failures and 78 errors occur ondeveloptoo; they are Warp tests whose C++ runtime can't build in my environment.🤖 Generated with Claude Code