Skip to content

Conversation

@guitargeek
Copy link
Contributor

@guitargeek guitargeek commented Nov 30, 2025

The Python package onnx 1.19.0 has a bug that makes this version unusable: onnx/onnx#7249. In that case, we have to disable the "TestSofieModels" and "TestRModelParserPyTorch" tests, which import onnx indirectly via torch.onnx.

We should also consider to require onnx!=1.19.1 in our requirements.txt in the future, so our users don't face similar trouble from exporting PyTorch models to onnx. But this should only be done once we are sure that it can also be installed on macOS without breaking something else.

Closes #20571.

There is also a followup to guitargeek@10f28b6, where I used a random keras version in a check in order to make the CI configuration at the time pass. In fact, keras=>3.5 also didn't work, as we see now after updating the macOS runners from Keras 2 to Keras 3.

It could actually turn out that keras>=3 is not supported at all, so we should be prepared to lower the maximum supported Keras version even further. But for now we don't know, as no platform has keras>=3.0&&keras<3.5 installed.

@guitargeek guitargeek self-assigned this Nov 30, 2025
@guitargeek guitargeek requested a review from bellenot as a code owner November 30, 2025 10:37
@guitargeek guitargeek changed the title [tmva][sofie] Only run tests that require onnx if onnx>=1.19.1 [tmva][sofie] Require onnx>=1.19.1 for tests Nov 30, 2025
@guitargeek guitargeek force-pushed the onnx_version_check branch 3 times, most recently from 020734a to dfe76cc Compare November 30, 2025 11:58
@guitargeek guitargeek changed the title [tmva][sofie] Require onnx>=1.19.1 for tests [tmva][sofie] Require onnx!=1.19.0 for tests Nov 30, 2025
@guitargeek guitargeek force-pushed the onnx_version_check branch 2 times, most recently from 7332842 to 9a267f9 Compare November 30, 2025 13:05
@guitargeek guitargeek changed the title [tmva][sofie] Require onnx!=1.19.0 for tests [tmva][sofie] Require onnx!=1.19.0 and keras<3.5 for tests Nov 30, 2025
This is a followup to 10f28b6, where I used a random keras version
in a check in order to make the CI configuration at the time pass. In
fact, `keras=>3.5` also didn't work, as we see now after updating the
macOS runners from Keras 2 to Keras 3.

It could actually turn out that `keras>=3` is not supported at all, so
we should be prepared to lower the maximum supported Keras version even
further. But for now we don't know, as no platform has
`keras>=3.0&&keras<3.5` installed.
The Python package onnx 1.19.0 has a bug that makes this version unusable:
onnx/onnx#7249
In that case, we have to disable the "TestSofieModels" and
"TestRModelParserPyTorch" tests, which import onnx indirectly via
`torch.onnx`.

We should also consider to require `onnx!=1.19.1` in our
`requirements.txt` in the future, so our users don't face similar
trouble from exporting PyTorch models to onnx. But this should only be
done once we are sure that it can also be installed on macOS without
breaking something else.

Closes root-project#20571.
Copy link
Member

@vepadulano vepadulano left a comment

Choose a reason for hiding this comment

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

Thank you so much! It's nice to see the builds green :)

@github-actions
Copy link

Test Results

    22 files      22 suites   3d 16h 43m 43s ⏱️
 3 780 tests  3 780 ✅ 0 💤 0 ❌
81 208 runs  81 208 ✅ 0 💤 0 ❌

Results for commit c6c851a.

@guitargeek guitargeek merged commit 8a4a8cb into root-project:master Nov 30, 2025
30 checks passed
@guitargeek guitargeek deleted the onnx_version_check branch November 30, 2025 15:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Potential dependency conflict with Python requirements.txt

2 participants