onnxruntime: 1.16.3 -> 1.18.0 by jkachmar · Pull Request #319137 · NixOS/nixpkgs

jkachmar · 2024-06-11T21:12:52Z

Description of changes

version bump; release notes for versions between 1.16.3 & 1.18.0:

i haven't tested any of the CUDA stuff.

maintainers: @puffnfresh @ck3d @cbourjau

possibly interested parties from #309015: @wexder @madsmtm

Things done

Add a 👍 reaction to pull requests you find important.

jkachmar · 2024-06-11T21:13:20Z

pkgs/development/libraries/onnxruntime/default.nix

i'm surprised flatbuffers works given that it should be a clear version mismatch

Well, see #330416

jkachmar · 2024-06-11T21:15:45Z

pkgs/development/libraries/onnxruntime/default.nix

need to support #include <locale> in onnxruntime/core/providers/cpu/text/string_normalizer.cc (& probably elsewhere).

without this a bunch of ContribOpTests fail (StringNormalizer* & WordConvEmbedding*)

jkachmar · 2024-06-11T21:17:10Z

pkgs/development/libraries/onnxruntime/default.nix

probably cleaner to factor out pytorch_cpuinfo rev & share that between the source & the clog version string.

also even though pytorch_clog doesn't seem to use gbenchmark it'll fail without -DUSE_SYSTEM_GOOGLEBENCHMARK=ON so we have to leave it there

jkachmar · 2024-06-11T21:19:00Z

pkgs/development/libraries/onnxruntime/default.nix

i feel like flatbuffers should fail given issues like microsoft/onnxruntime#14981; maybe the local build i tested out was flawed and it's actually not gonna work in CI

jkachmar · 2024-06-11T21:20:59Z

pkgs/development/libraries/onnxruntime/nvcc-gsl.patch

i think this patch might be unnecessary, given the comment in onnxruntime/default.nix on the line where this patch is included

jkachmar · 2024-06-11T21:45:54Z

pkgs/development/libraries/onnxruntime/default.nix

  onnx = fetchFromGitHub {
    owner = "onnx";
    repo = "onnx";
-    rev = "refs/tags/v1.14.1";
-    hash = "sha256-ZVSdk6LeAiZpQrrzLxphMbc1b3rNUMpcxcXPP8s/5tE=";
+    rev = "refs/tags/v1.16.1";
+    hash = "sha256-I1wwfn91hdH3jORIKny0Xc73qW2P04MjkVCgcaNnQUE=";
  };


why are we vendoring onnx? it looks like there's an open MR to bump to 1.16 which would be compatible here #299313

@acairncross, since you're listed as the onnx maintainer

IIRC, onnxruntime is loosely correlated to onnx (the python package) so the don't have to be in lockstep. Although, I would probably prefer that we try to move to a world where we don't vendor a local copy.

so i can open a PR updating onnx to 1.16.1 & then reference the source here; is there a preference for how to structure that?

i guess python3Packages is already an argument, so it could just be python3Packages.onnx_1_16.src & some comment or assertion indicating that we want compatibility checks in a similar fashion to how other packages with source dependencies like this are structured?

There's a tradeoff, as now if you bump onnx, you have to be concerned about onnxruntime. And it's a matter of if that should be the case.

jonringer · 2024-06-11T23:01:41Z

pkgs/development/libraries/onnxruntime/0001-eigen-allow-dependency-injection.patch

 -    FetchContent_Populate(eigen)
 -    set(eigen_INCLUDE_DIRS  "${eigen_SOURCE_DIR}")
 +    FetchContent_MakeAvailable(eigen)
 +    add_library(eigen ALIAS Eigen3::Eigen)
-+
 +    # Onnxruntime doesn't always use `eigen` as a target in
 +    # `target_link_libraries`, sometimes it just uses
 +    # `target_include_directories`:
 +    get_target_property(eigen_INCLUDE_DIRS Eigen3::Eigen INTERFACE_INCLUDE_DIRECTORIES)


I really need to upstream a -DPREFER_SYSTEM_LIBS type of switch..... tired of having to carry around patches which disable network behaviors.

jonringer · 2024-06-11T23:07:41Z

pkgs/development/libraries/onnxruntime/default.nix

  onnx = fetchFromGitHub {
    owner = "onnx";
    repo = "onnx";
-    rev = "refs/tags/v1.14.1";
-    hash = "sha256-ZVSdk6LeAiZpQrrzLxphMbc1b3rNUMpcxcXPP8s/5tE=";
+    rev = "refs/tags/v1.16.1";
+    hash = "sha256-I1wwfn91hdH3jORIKny0Xc73qW2P04MjkVCgcaNnQUE=";
  };


IIRC, onnxruntime is loosely correlated to onnx (the python package) so the don't have to be in lockstep. Although, I would probably prefer that we try to move to a world where we don't vendor a local copy.

jkachmar · 2024-06-14T17:53:06Z

okay looks like there are some failures with clog so i'm going to have to actually test this out more on nixpkgs-24.05:

make[2]: *** [CMakeFiles/clog-test.dir/build.make:76: CMakeFiles/clog-test.dir/test/clog.cc.o] Error 1
make[1]: *** [CMakeFiles/Makefile2:111: CMakeFiles/clog-test.dir/all] Error 2
make: *** [Makefile:146: all] Error 2
error: builder for '/nix/store/2kh962g19v3yglcadnjy2i71yaabivzz-clog-3c8b153.drv' failed with exit code 2;
       last 10 log lines:
       > /nix/store/8l47dmnifglj4lrwxsjf08jf1qzmbh1m-gtest-1.14.0-dev/include/gtest/gtest.h: At global scope:
       > /nix/store/8l47dmnifglj4lrwxsjf08jf1qzmbh1m-gtest-1.14.0-dev/include/gtest/gtest.h:302:30: error: 'std::enable_if_t' has not been declared
       >   302 |   template <typename T, std::enable_if_t<std::is_convertible<T, int64_t>::value,
       >       |                              ^~~~~~~~~~~
       > /nix/store/8l47dmnifglj4lrwxsjf08jf1qzmbh1m-gtest-1.14.0-dev/include/gtest/gtest.h:302:41: error: expected '>' before '<' token
       >   302 |   template <typename T, std::enable_if_t<std::is_convertible<T, int64_t>::value,
       >       |                                         ^
       > make[2]: *** [CMakeFiles/clog-test.dir/build.make:76: CMakeFiles/clog-test.dir/test/clog.cc.o] Error 1
       > make[1]: *** [CMakeFiles/Makefile2:111: CMakeFiles/clog-test.dir/all] Error 2
       > make: *** [Makefile:146: all] Error 2
       For full logs, run 'nix log /nix/store/2kh962g19v3yglcadnjy2i71yaabivzz-clog-3c8b153.drv'.

jonringer · 2024-06-14T17:59:14Z

maybe a CXX_STANDARD issue?

jkachmar · 2024-06-14T18:37:43Z

ah yeah might be; clog tests set CXX 11. i disabled that & it looks like there's another error with this:

CMake Error: install(EXPORT "abslTargets" ...) includes target "test_allocator" which requires target "gmock" that is not in any export set.

...which i think is resolved by applying this patch from upstream which isn't in 1.18.0...?

i guess i didn't run into this at work b/c we're on older versions of gtest & abseil-cpp (from 23.11); rebuilding onnxruntime & i'll push changes if it passes i guess.

Release notes: - https://github.com/microsoft/onnxruntime/releases/tag/v1.17.0 - https://github.com/microsoft/onnxruntime/releases/tag/v1.17.1 - https://github.com/microsoft/onnxruntime/releases/tag/v1.17.3 - https://github.com/microsoft/onnxruntime/releases/tag/v1.18.0

jkachmar commented Jun 11, 2024

View reviewed changes

jkachmar force-pushed the onnxruntime-1_18_0 branch from d512061 to 2423bcd Compare June 11, 2024 21:29

jkachmar commented Jun 11, 2024

View reviewed changes

ofborg bot requested review from ck3d, jonringer and puffnfresh June 11, 2024 21:47

ofborg bot added 10.rebuild-darwin: 11-100 This PR causes between 11 and 100 packages to rebuild on Darwin. 10.rebuild-linux: 11-100 This PR causes between 11 and 100 packages to rebuild on Linux. labels Jun 11, 2024

jonringer reviewed Jun 11, 2024

View reviewed changes

jkachmar closed this Jun 14, 2024

jkachmar force-pushed the onnxruntime-1_18_0 branch from 2423bcd to 0f76878 Compare June 14, 2024 19:01

jkachmar requested review from IvarWithoutBones, Mic92, RaitoBezarius, SuperSandro2000, adamcstephens, bendlas, figsoda, lilyinstarlight, mweinelt, winterqt and zowoq as code owners June 14, 2024 19:01

NixOS locked and limited conversation to collaborators Jun 14, 2024

Uh oh!

Conversation

jkachmar commented Jun 11, 2024

Description of changes

Things done

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jkachmar commented Jun 14, 2024

Uh oh!

jonringer commented Jun 14, 2024

Uh oh!

jkachmar commented Jun 14, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants