-
Notifications
You must be signed in to change notification settings - Fork 87
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
NeVa - ValueError: <object> had an unexpected type <class 'object'> #717
Comments
Minimal Repro: import torch
import thunder
def f(ids):
for ix, t in enumerate(ids):
pass
return ids
jf = thunder.jit(f)
ids = torch.randn(2, 2).to(torch.long)
jf(ids) |
Great repro, I think we would want
Does that make sense? |
I tried adding lookaside for diff --git a/thunder/core/jit_ext.py b/thunder/core/jit_ext.py
index ac9e127..2da42d7 100644
--- a/thunder/core/jit_ext.py
+++ b/thunder/core/jit_ext.py
@@ -890,6 +890,18 @@ def _general_jit_named_buffers_lookaside(obj: Any, *args, **kwargs):
model, model.named_buffers, model.get_buffer, *unwrapped_args, **unwrapped_kwargs
)
+@general_jit_lookaside(torch.Tensor.__iter__)
+def _general_tensor_iter_lookaside(obj: Any, *args, **kwargs):
+
+ # NOTE: This will be interpreted.
+ def _tensor_iter_impl(t):
+ for t_slice in t.unbind():
+ yield t_slice
+
+ pr = ProvenanceRecord(PseudoInst.LOOKASIDE, inputs=[wrap_const(torch.Tensor.__iter__).provenance])
+
+ return _interpret_call(_tensor_iter_impl, wrap(unwrap(obj), pr))
+
@general_jit_lookaside(torch.autograd.function.Function.apply.__func__)
def _general_jit_torch_autograd_function_apply_lookaside(obj: Any, *args, **kwargs):
But it never gets called. On printing what Adding diff --git a/thunder/core/proxies.py b/thunder/core/proxies.py
index df6dce4..b440091 100644
--- a/thunder/core/proxies.py
+++ b/thunder/core/proxies.py
@@ -1340,6 +1340,9 @@ class TensorProxy(Proxy, TensorProxyInterface):
method = resolve_method("getitem", self, key)
return method(self, key)
+ def __iter__(self):
+ return iter(self.unbind())
+
#
# Elementwise unary operators
# What do you think about this (or maybe there is still a way with lookaisde)? |
Ooops, sorry, @kshitij12345 , I accidentally stepped on your feet... But I am not sure about my approach. Is it fine to have a lookside for TensorProxies? |
Ah, no worries, as long as the issue is fixed :) Thanks for looking into this.
@t-vi what are your thoughts? |
Seems good to have one. The reason to do it as a lookaside is to not handle iter objects in the trace. |
But yeah, if defining the iter on the tensorproxy works, lets just have that, I think it might be the same result just with a slightly different execution model. We should comment the itermethod. What is the trace we get from that? |
@t-vi , something like this
|
I'd say not overly pretty but not too terrible. |
NOTE: For minimal repro - see comment below
Full Log - error.log
(Steps to repro are same from #678 and copied from there except addition of megatron_core commit details in environment)
To Repro -
Note you'll need the referenced ./data directory; ping @tfogal privately for now.
Environment
cc @tfogal
The text was updated successfully, but these errors were encountered: