Skip to content

Commit f2bc7b7

Browse files
committed
[tmva][sofie] Require onnx>=1.19.1 for tests
In commit 178a9f9, I activated several SOFIE tests by moving the SOFIE PyTorch and Keras parsers out of `tmva/pymva`, because SOFIE and PyMVA are unrelated and we lost test coverage for these SOFIE parsers, because `tmva-pymva` is always disabled. But as reported in #20571, we now see SOFIE test failures on macOS because it is not possible to fully resolve the required Python environment. The problem was that we implicitly required `onnx>=1.19.1` for the SOFIE tests to pass, but this can't be installed on macOS because the transient update of `ml_dtypes` is conflicting with the current version of the `tensorflow` package. To get out of this, this commit suggests to only enable the SOFIE PyTorch/Keras parser tests if `onnx>=1.19.1`, with a warning otherwise. This is the solution with the least test coverage regression. There is even no coverage regression at all, if you compare to the state before 178a9f9 a few days ago. 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. But this should only be done once we are sure that it can also be installed on macOS without breaking something else. Closes #20571.
1 parent 1df6d55 commit f2bc7b7

File tree

1 file changed

+29
-5
lines changed

1 file changed

+29
-5
lines changed

tmva/sofie/test/CMakeLists.txt

Lines changed: 29 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -108,8 +108,32 @@ endif()
108108
# Look for needed Python modules
109109
ROOT_FIND_PYTHON_MODULE(torch)
110110
ROOT_FIND_PYTHON_MODULE(keras)
111+
ROOT_FIND_PYTHON_MODULE(onnx)
111112

112-
if (ROOT_TORCH_FOUND)
113+
114+
# Some tests need to be disabled when the onnx version is too low, because then
115+
# onnx is not compatible with certain PyTorch or TensorFlow versions.
116+
# See also: https://github.com/root-project/root/issues/20571
117+
#
118+
# TODO: We can't enforce this minimum onnx version in the global
119+
# requirements.txt yet, because the transient update of ml_dtypes is
120+
# conflicting with the current version of the tensorflow package on the macOS
121+
# runners. Once we have installed onnx>=1.19.1 on all of our CI nodes that
122+
# install onnx, we should also consider requiring this minimum version from our
123+
# users, se they don't face bad surprises like:
124+
# https://github.com/onnx/onnx/issues/7249
125+
set(minimum_onnx_version "1.19.1")
126+
if (NOT DEFINED ROOT_ONNX_VERSION)
127+
message(WARNING "onnx found, but version unknown — cannot verify compatibility.")
128+
set(minimum_onnx_found TRUE) # In this case we assume it is of the right version
129+
elseif (ROOT_ONNX_VERSION VERSION_LESS ${minimum_onnx_version})
130+
message(WARNING "onnx version ${ROOT_ONNX_VERSION} found, but we only support onnx>=${minimum_onnx_version} for compatibility reasons. Some TMVA-SOFIE tests will be disabled.")
131+
set(minimum_onnx_found FALSE)
132+
else()
133+
set(minimum_onnx_found TRUE)
134+
endif()
135+
136+
if (minimum_onnx_found AND ROOT_TORCH_FOUND)
113137
configure_file(Conv1dModelGenerator.py Conv1dModelGenerator.py COPYONLY)
114138
configure_file(Conv2dModelGenerator.py Conv2dModelGenerator.py COPYONLY)
115139
configure_file(Conv3dModelGenerator.py Conv3dModelGenerator.py COPYONLY)
@@ -129,8 +153,8 @@ if (ROOT_TORCH_FOUND)
129153
endif()
130154
endif()
131155

132-
# Any reatures that link against libpython are disabled if built with tpython=OFF
133-
if (tpython AND ROOT_TORCH_FOUND AND BLAS_FOUND)
156+
# Any features that link against libpython are disabled if built with tpython=OFF
157+
if (minimum_onnx_found AND tpython AND ROOT_TORCH_FOUND AND BLAS_FOUND)
134158
configure_file(generatePyTorchModelClassification.py generatePyTorchModelClassification.py COPYONLY)
135159
configure_file(generatePyTorchModelMulticlass.py generatePyTorchModelMulticlass.py COPYONLY)
136160
configure_file(generatePyTorchModelRegression.py generatePyTorchModelRegression.py COPYONLY)
@@ -151,8 +175,8 @@ if (tpython AND ROOT_TORCH_FOUND AND BLAS_FOUND)
151175
endif()
152176

153177

154-
# Any reatures that link against libpython are disabled if built with tpython=OFF
155-
if (tpython AND ROOT_KERAS_FOUND AND BLAS_FOUND)
178+
# Any features that link against libpython are disabled if built with tpython=OFF
179+
if (minimum_onnx_found AND tpython AND ROOT_KERAS_FOUND AND BLAS_FOUND)
156180

157181
set(unsupported_keras_version "3.10.0")
158182

0 commit comments

Comments
 (0)