Skip to content

Conversation

@vawale
Copy link
Contributor

@vawale vawale commented Aug 3, 2025

Adds CMakeLists.txt file for cmake build of onnx app. The build logic is kept same as the one from corresponding Makefile.

Fixes #8737

vawale added 2 commits August 3, 2025 15:28
`py::array::itemsize()` returns 0 for tests in `model_test.py`. I could
not really figure out why from pybind11 docs. Using `request().itemsize`
on the other hand always returns the correct item size.
Adds CMakeLists.txt file for cmake build of onnx app. The build logic is
kept same as the one from corresponding Makefile.
@alexreinking alexreinking self-requested a review August 3, 2025 14:41
@alexreinking
Copy link
Member

Thanks for taking a crack at this! We're currently performing some maintenance on the build bots, but I will take a look at this when we're done.

@alexreinking
Copy link
Member

alexreinking commented Aug 5, 2025

Going to see what the buildbots think. Looking at recent logs, it seems protoc was missing from the macOS workers (at least), so that scenario hasn't been tested in a while. I'm curious to see what happens with the new venv support I added to the buildbots.

There probably needs to be a -force_load or equivalent case, but TBQH I'm not convinced that will work with the newer machO formats. I'll bet this app needs to be passed the path to the autoscheduler library like the other apps do.

Copy link
Member

@alexreinking alexreinking left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Still waiting on buildbots, but this is what stood out to me on a first pass.

@alexreinking
Copy link
Member

Some of the buildbots were missing protoc. Now I see the following errors:

In file included from model.cpp:7:
./denormal_disabler.h:38:18: error: private field 'csr_' is not used [-Werror,-Wunused-private-field]
    unsigned int csr_ = 0;
                 ^
./denormal_disabler.h:39:10: error: private field 'need_restore_' is not used [-Werror,-Wunused-private-field]
    bool need_restore_ = false;
         ^
2 errors generated.
make[2]: *** [/Users/halidenightly/build_bot/worker/halide-testbranch-main-llvm_main-arm-64-osx-make/halide-build/bin/apps/onnx/bin/host/model_cpp.cpython-313-darwin.so] Error 1
make[1]: *** [onnx_test_app] Error 1
make: *** [test_apps] Error 2

@vawale
Copy link
Contributor Author

vawale commented Aug 8, 2025

Thanks for the review and the follow up fixes. I will look into addressing the comments soon :)

Some of the buildbots were missing protoc. Now I see the following errors:

Ah okay. Should I skip building this app if protoc is not available?

@alexreinking
Copy link
Member

@vawale -- while playing with this, I discovered a bunch more changes that needed to be made in other parts of the project. Assuming the buildbots like it at this point, I'm happy to merge.

@alexreinking
Copy link
Member

New failures on the mac bots because WebGPU is missing. This is because they're running under launchd now and don't have the environment variables set. This is brittle, so I'm going to do this The Right Way...

Homebrew/homebrew-core#232863

@alexreinking
Copy link
Member

This is brittle, so I'm going to do this The Right Way...

Turns out this is too hard. See #8714 .

@alexreinking
Copy link
Member

@vawale -- I see that the halide_as_onnx_backend_test is timing out (and that subtests are failing). Does it work for you?

https://buildbot.halide-lang.org/master/#/builders/265/builds/30

@alexreinking
Copy link
Member

Seems like halide_as_onnx_backend_test is failing because Adams2019 is thinking for too long.

@vawale
Copy link
Contributor Author

vawale commented Aug 14, 2025

Seems like halide_as_onnx_backend_test is failing because Adams2019 is thinking for too long.

Yes, I observe the same. The auto-scheduler takes too long.

@alexreinking
Copy link
Member

Despite all the workflows failing... none of them are related to this change. Thanks so much for your help!

@alexreinking alexreinking merged commit 21d2587 into halide:main Aug 14, 2025
4 of 19 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add testing for onnx

2 participants