pip install reference_model and use pybind #7077
Conversation
Erik-Lundell
added
partner: arm
🔗 Helpful Links🧪 See artifacts and rendered test results at hud.pytorch.org/pr/pytorch/executorch/7077
Note: Links to docs will display an error until the docs builds have been completed. ✅ No FailuresAs of commit 5105a01 with merge base 22a75be ( This comment was automatically generated by Dr. CI and updates every 15 minutes. |
facebook-github-bot
added
the
CLA Signed
|
@digantdesai pls have a look at this change. We've been debating it back and forth internally and it would be good to get your opinion. |
The reference model is pip installed in setup.sh. Also install vela similarily. Since the installation contains serialization_lib, we don't have to include it as a package in Executorch's setup.py. The serialization_lib is still needed as a submodule in the arm backend to find the tosa.fbs for deserialization. Change-Id: I24fff6c00a3961444de5d878ab169d5ba4c9156d
Erik-Lundell
force-pushed
the
pip-refmodel
branch
from
c08da9c to
d5dcc26
Compare
| # any checking of compatibility. | ||
| dbg_fail(node, tosa_graph, artifact_path) | ||
|
|
||
| if len(input_order) > 0: |
There was a problem hiding this comment.
Result of an incorrect merge, will add back. Good catch, thanks!
backends/arm/test/common.py
Outdated
|
|
||
| if not os.path.exists(intermediate_path): | ||
| os.makedirs(intermediate_path, exist_ok=True) | ||
| if custom_path is not None and not os.path.exists(custom_path): |
There was a problem hiding this comment.
why do we need the latter condition with exist_ok=True?
There was a problem hiding this comment.
We don't, this was probably just inherited from old code. But good point.
| .tosa_compile_spec(tosa_version) | ||
| .set_permute_memory_format(permute_memory_to_nhwc) | ||
| .dump_intermediate_artifacts_to(intermediate_path) | ||
| .dump_intermediate_artifacts_to(custom_path) |
There was a problem hiding this comment.
I guess OK to call with None?
There was a problem hiding this comment.
Should be ok. If you run on FVP and have custom_path=None you will get a crash, but with a explanatory error message.
examples/arm/setup.sh
Outdated
|
|
||
| # reference_model flatbuffers version clashes with Vela. | ||
| # go with Vela's since it newer. | ||
| # Could cause issues down the line, beware.. |
There was a problem hiding this comment.
what is the solution? Where is the reference to Vela?
There was a problem hiding this comment.
Vela is working on widening their flatbuffer requirement, should be done within a couple of weeks
| ) | ||
| else: | ||
| intermediate_path = custom_path | ||
| custom_path = maybe_get_tosa_collate_path() |
There was a problem hiding this comment.
@Erik-Lundell - keep unrelated related changes in different commits, same PR is OK.
There was a problem hiding this comment.
Since we don't have to save files unless we want to with this change, we don't need to create a temporary dir when custom_path is not explicitly set -> I viewed this as a related change.
Signed-off-by: Erik Lundell <[email protected]> Change-Id: If733617374683765bf1d49ff8f64e4c7ab9bc42d
|
@digantdesai are u ok with the changes. I suspect that the docker-builds have pushed an image to the registry including this change. Hence all our CI fails. |
|
To avoid docker issue, lets merge this and then we can discuss async. |
4 participants
The reference model is pip installed in setup.sh.
Also install vela similarly.
Since the installation contains serialization_lib, we don't have to include it as a package
in Executorch's setup.py. The serialization_lib
is still needed as a submodule in the arm backend
to find the tosa.fbs for deserialization.
Change-Id: I24fff6c00a3961444de5d878ab169d5ba4c9156d