Closed IvanYashchuk closed 3 weeks ago
Thank you @IvanYashchuk The underlying issue is "need lookaside for zip in interpreter".
Seems that this is a good issue for someone who wants to take a look at our great Python interpreter (thunder/core/interpreter.py
), it's not trivial but should be relatively self-contained.
@t-vi what would be a similar lookaside to start from for anyone wanting to approach this?
I would I assume that functools.reduce
is not that bad to look at, specifically because of the test coverage (generic iterables, custom iterables, etc.).
Doesn't the error message NotImplementedError: unpacking from OPAQUE <slot wrapper '__next__' of 'zip' objects>
mean that the problem is in the next
interpretation and not in zip
?
@t-vi to me it looks like there some errors in the unpacking more than the zip, might it be the opaque ModuleList container?
I would like to participate in the solution for this issue. I have some knowledge of ast library and worked on clang static analyzer before.
Hi @Nachiket18 ,
great!
So the main issue here seems to be that we would want the Thunder Python Interpreter to be able to "see through" zip (i.e. link the things that are yielded by the zip iteration with the arguments to zip).
I think it should be as easy as implementing a zip function in pure Python and putting it in a lookaside.
If you look at def _enumerate_lookaside(obj: Iterable, start: int = 0):
in thunder/core/interpreter.py
, that would give you a good idea of how to do it. Testing should be added to thunder/tests/test_interpreter.py
for the new zip to give the same as the old. The repro @IvanYashchuk has in the issue could go in test_jit_general.py
.
Please let me know if there is anything else you need to get started. Also don't hesitate to reach out if you find you're stuck somewhere.
Hello @t-vi - Thanks for explanation. Sorry for late response. We were stuck at job and school work.
@UltraArceus3 and I wrote some code on our fork along with test cases and it ran to success.
https://github.com/Nachiket18/lightning-thunder/commit/bc1fafe411b8f52ad9ce533a638ee01d6fd908e6
Would like to know your thoughts on the same.
@t-vi - I was wondering if you could review the code that we wrote. Let us know if we are making some mistake and needing some correction.
Looks very reasonable, would love to see a PR.
🐛 Bug
Here's a simplified version of LitGPT's LLaMAMoE without data-dependent shapes and it fails somewhere in the general jit:
To reproduce:
raises: