Use python bindings for tests #795
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
makslevental
force-pushed
the
makslevental/python-bindings-tests
branch
30 times, most recently
from
48830a5
to
c4ac2ba
Compare
makslevental
force-pushed
the
makslevental/python-bindings-tests
branch
from
c1bdeb0
to
df692aa
Compare
makslevental
force-pushed
the
makslevental/python-bindings-tests
branch
from
df692aa
to
12787ce
Compare
jtuyls
reviewed
Oct 1, 2024
jtuyls
reviewed
Oct 1, 2024
jtuyls
reviewed
Oct 1, 2024
| target_compile_definitions(${_core_lib} | ||
| PUBLIC | ||
| -DBOOST_BIND_GLOBAL_PLACEHOLDERS | ||
| $<$<CONFIG:Debug>:-DXRT_VERBOSE>) |
There was a problem hiding this comment.
Is this for debugging purposes?
There was a problem hiding this comment.
Should we enable it by default?
There was a problem hiding this comment.
Nah it's too "loud".
There was a problem hiding this comment.
Sorry, I was assuming it was enabled by default now. If it's not, that's good
Co-authored-by: Jorn Tuyls <[email protected]>
makslevental
force-pushed
the
makslevental/python-bindings-tests
branch
from
a936db5
to
fc48087
Compare
|
Off the top of my head things which we need to reach parity, hopefully we can reuse what's already done in cpu_comparison/run.py:
Also, can you/someone please at some point:
|
None have been moved, some have been duplicated. |
daveliddell
pushed a commit
to daveliddell/iree-amd-aie
that referenced
this pull request
Oct 7, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR begins the migration to
pytestfor all of oure2etests using IREE's python bindings for both the compiler and the runtime. I've tested it thoroughly (by runningrun_matmul.shandcpu_comparison/run.py10050 times).Two major changes were required to make this happen:
AMDAIEOptionsso that they're registered at plugin load time. This was necessary for enablingSession.set_flagsin Python. Going forward we should make sure to put all CLI args in the same place;1536x2048x1536that was prior disabled for Windows but is now disabled entirely. I don't know what the connection is (I wasn't able to track it down) but since every other test passes, including the 1000 run test, and even after repeating overall10050 runs, I think it's safe to conclude there's something wrong with that shape rather than the HAL.The migration isn't complete because the bindings and the VM itself don't support bf16 yet; I will send them a PR soon.
Note, in its current state the test scripts run
10050 times. I will remove this before landing.TODO (for the next iteration): right now
repeat_runsdoesn't work because of how the XRT command buffer works (it's free'ed after every dispatch and with it thexrt::kernelandxrt::hw_context). Likely I'm doing something wrong (possibly using the wrong command buffer model) so I think it should be fixable.