fix: enable compilation with ONNX 1.13 #1835

gagnonlg · 2023-02-07T12:38:37Z

Simple PR to fix a compilation error against ONNX runtime 1.13.1. Basically, the Ort::Session::Get{Input,Output}Name functions got removed starting from 1.13, and we have the use the Ort::Session::Get{Input,Output}NameAllocated versions which were introduced in 1.12.

One implication is that this PR introduces a lower bound of 1.12.0 on the ONNX runtime version.

paulgessinger · 2023-02-07T12:40:45Z

Let me tag @benjaminhuth and @Corentin-Allaire. Thoughts?

benjaminhuth · 2023-02-07T12:47:09Z

Very good to stay compatible with newer ONNX versions. However, I see two points before this gets merged:

I think we need to update our CI image to a newer ONNX version
This probably also has implications for the Exa.TrkX plugin with the (currently unused) ONNX backend. It can be enabled via the ACTS_EXATRKX_ENABLE_ONNX cmake flag.

Corentin-Allaire · 2023-02-07T12:48:44Z

I have done those exact same change (plus some other) in my own branch. I general I think we should discuss how Onnx is implemented in Acts right now (with the current implementation you need to create a new class that inherit from OnnxRuntimeBase each time you import a new network but we can discuss this once I open a PR for the ambiguity resolver

gagnonlg · 2023-02-07T12:50:44Z

Good point about Exa.Trk, tagging @xju2 -- do you know if your pipeline works with OnnxRuntime >= 1.12.0?

gagnonlg · 2023-02-07T12:53:40Z

I general I think we should discuss how Onnx is implemented in Acts right now (with the current implementation you need to create a new class that inherit from OnnxRuntimeBase each time you import a new network but we can discuss this once I open a PR for the ambiguity resolver

@Corentin-Allaire Do you mean that in your ambi. solver work, you rely on changing the current implementation? If so, maybe it'd be worth it to merge that part in sooner (or start the discussion as you suggest) so as to minimize potential divergence? just my .02$

Corentin-Allaire · 2023-02-07T12:59:05Z

I general I think we should discuss how Onnx is implemented in Acts right now (with the current implementation you need to create a new class that inherit from OnnxRuntimeBase each time you import a new network but we can discuss this once I open a PR for the ambiguity resolver

@Corentin-Allaire Do you mean that in your ambi. solver work, you rely on changing the current implementation? If so, maybe it'd be worth it to merge that part in sooner (or start the discussion as you suggest) so as to minimize potential divergence? just my .02$

@gagnonlg My main change for now but it is not definitive was to switch runONNXInference to public that way your algorithm can have a Acts::OnnxRuntimeBase member that load the model of your choice instead of writing a new class for each model you import as it is done right now.

gagnonlg · 2023-02-07T13:02:05Z

@gagnonlg My main change for now but it is not definitive was to switch runONNXInference to public that way your algorithm can have a Acts::OnnxRuntimeBase member that load the model of your choice instead of writing a new class for each model you import as it is done right now.

I see, seems like that would be a backwards compatible change if that decision is made so there's no hurry, I guess

What do you mean by .02$ ? Is it a typo or something I don't understand ?

Oh, it's literally "my 2 cents", an idiom that basically means "in my opinion", i.e. feel free to disagree

Corentin-Allaire · 2023-02-07T13:03:37Z

@gagnonlg My main change for now but it is not definitive was to switch runONNXInference to public that way your algorithm can have a Acts::OnnxRuntimeBase member that load the model of your choice instead of writing a new class for each model you import as it is done right now.

I see, seems like that would be a backwards compatible change if that decision is made so there's no hurry, I guess

What do you mean by .02$ ? Is it a typo or something I don't understand ?

Oh, it's literally "my 2 cents", an idiom that basically means "in my opinion", i.e. feel free to disagree

I got the 2 cents afterward, I knew the expression but it this the first I see it written that way !

codecov · 2023-02-07T13:13:10Z

Codecov Report

Merging #1835 (a63f605) into main (97a8458) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##             main    #1835   +/-   ##
=======================================
  Coverage   49.50%   49.50%           
=======================================
  Files         407      407           
  Lines       22646    22646           
  Branches    10334    10334           
=======================================
  Hits        11210    11210           
  Misses       4247     4247           
  Partials     7189     7189

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

github-actions · 2023-02-07T13:14:27Z

📊 Physics performance monitoring for `a63f605`

Full report
Seeding: seeded, truth estimated, orthogonal
CKF: seeded, truth smeared, truth estimated, orthogonal
IVF: seeded, truth smeared, truth estimated, orthogonal
Ambiguity resolution: seeded, orthogonal
Truth tracking
Truth tracking (GSF)

Vertexing

Vertexing vs. mu

IVF seeded

IVF truth_smeared

IVF truth_estimated

IVF orthogonal

Seeding

Seeding seeded

Seeding truth_estimated

Seeding orthogonal

CKF

CKF seeded

CKF truth_smeared

CKF truth_estimated

CKF orthogonal

Ambiguity resolution

seeded

Truth tracking (Kalman Filter)

Truth tracking

Truth tracking (GSF)

Truth tracking

benjaminhuth · 2023-02-09T13:08:22Z

We have a new image now, if you replace ghcr.io/acts-project/ubuntu2004_exatrkx:v30 with ghcr.io/acts-project/ubuntu2004_exatrkx:v31 in .gitlab-ci.yaml, then we should see if it acutally builds @gagnonlg

gagnonlg · 2023-02-09T13:11:19Z

@benjaminhuth awesome, let's try it now

gagnonlg · 2023-02-09T14:21:24Z

@benjaminhuth are you sure it's v31? Looks like OnnxRuntime 1.10 is still being picked up

benjaminhuth · 2023-02-09T14:22:26Z

Yeah I also noticed it. I'll check what I messed up

benjaminhuth · 2023-02-09T14:24:49Z

Ah I mixed the numbers. It is release 34, not 31. Sorry!

gagnonlg · 2023-02-09T14:30:39Z

Ah I mixed the numbers. It is release 34, not 31. Sorry!

No worries, it's an easy fix!

benjaminhuth · 2023-02-09T17:17:30Z

Hmm this time the error is

section_end:1675953156:prepare_executor ERROR: Job failed: failed to pull image "ghcr.io/acts-project/ubuntu2004_exatrkx:v34" with specified policies
[always]: write /var/lib/docker/tmp/GetImageBlob814379636: no space left on device (manager.go:203:71s)

Do we need to reduce somehow the size of the image @paulgessinger ?

paulgessinger · 2023-02-09T17:18:52Z

No I guess the disk is simply full on the VM. In principle I clear the image storage regularly but you can try rerunning which hopefully ends up on another VM.

xju2 · 2023-02-09T17:59:26Z

Hi @gagnonlg , the ONNX models were not working because some operations were not supported by that time. Maybe the new version will fix those. ;-)

gagnonlg · 2023-02-10T08:28:10Z

@benjaminhuth from the ci-bridge:

$ python3 src/Examples/Scripts/Python/exatrkx.py onnx
Traceback (most recent call last):
  File "src/Examples/Scripts/Python/exatrkx.py", line 5, in <module>
    import acts.examples
  File "/builds/acts/ci-bridge/build/python/acts/__init__.py", line 7, in <module>
    from .ActsPythonBindings import *
ImportError: libonnxruntime.so.1.13.1: cannot open shared object file: No such file or directory

benjaminhuth · 2023-02-10T10:00:49Z

Nice that it builds without errors!

The test workflow still runs on the v30 image, you would also need to update it to v34 (also in .gitlab-ci.yaml)
But then I'm confident it runs through!

gagnonlg · 2023-02-10T10:57:36Z

@benjaminhuth of course, I need to bump the test image as well 🤡

gagnonlg · 2023-02-10T13:19:05Z

All fine now, I think this can go in. @benjaminhuth can you approve?

Simple PR to fix a compilation error against ONNX runtime 1.13.1. Basically, the `Ort::Session::Get{Input,Output}Name` functions got removed starting from 1.13, and we have the use the `Ort::Session::Get{Input,Output}NameAllocated` versions which were introduced in 1.12. One implication is that this PR introduces a lower bound of 1.12.0 on the ONNX runtime version.

gagnonlg added 2 commits February 7, 2023 12:53

Fix compilation with ONNX 13

545326a

Add OnnxRuntime API version check in cmake

09b01d2

gagnonlg added Bug Something isn't working Component - Plugins Affects one or more Plugins Impact - Minor Nuissance bug and/or affects only a single module labels Feb 7, 2023

gagnonlg added this to the next milestone Feb 7, 2023

benjaminhuth self-assigned this Feb 7, 2023

Bump exatrkx image

930d0d2

Bump exatrk image again

4df4a12

(empty) run CI again

fdbc682

Bump the test image as well

c2e4060

benjaminhuth approved these changes Feb 10, 2023

View reviewed changes

benjaminhuth added the automerge label Feb 10, 2023

Merge branch 'main' into onnx-13

a63f605

kodiakhq bot merged commit c2ef676 into acts-project:main Feb 10, 2023

github-actions bot removed the automerge label Feb 10, 2023

paulgessinger modified the milestones: next, v23.3.0 Feb 21, 2023

wdconinc mentioned this pull request Apr 20, 2023

acts: new versions 23.[3-5].0, 24.0.0, 25.0.[0-1], 26.0.0, 27.[0-1].0, 28.0.0 spack/spack#37055

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: enable compilation with ONNX 1.13 #1835

fix: enable compilation with ONNX 1.13 #1835

gagnonlg commented Feb 7, 2023

paulgessinger commented Feb 7, 2023 •

edited

Loading

benjaminhuth commented Feb 7, 2023 •

edited

Loading

Corentin-Allaire commented Feb 7, 2023 •

edited

Loading

gagnonlg commented Feb 7, 2023 •

edited

Loading

gagnonlg commented Feb 7, 2023 •

edited

Loading

Corentin-Allaire commented Feb 7, 2023 •

edited

Loading

gagnonlg commented Feb 7, 2023

Corentin-Allaire commented Feb 7, 2023

codecov bot commented Feb 7, 2023 •

edited

Loading

github-actions bot commented Feb 7, 2023 •

edited

Loading

benjaminhuth commented Feb 9, 2023

gagnonlg commented Feb 9, 2023

gagnonlg commented Feb 9, 2023

benjaminhuth commented Feb 9, 2023

benjaminhuth commented Feb 9, 2023 •

edited

Loading

gagnonlg commented Feb 9, 2023

benjaminhuth commented Feb 9, 2023

paulgessinger commented Feb 9, 2023

xju2 commented Feb 9, 2023

gagnonlg commented Feb 10, 2023

benjaminhuth commented Feb 10, 2023

gagnonlg commented Feb 10, 2023

gagnonlg commented Feb 10, 2023

fix: enable compilation with ONNX 1.13 #1835

fix: enable compilation with ONNX 1.13 #1835

Conversation

gagnonlg commented Feb 7, 2023

paulgessinger commented Feb 7, 2023 • edited Loading

benjaminhuth commented Feb 7, 2023 • edited Loading

Corentin-Allaire commented Feb 7, 2023 • edited Loading

gagnonlg commented Feb 7, 2023 • edited Loading

gagnonlg commented Feb 7, 2023 • edited Loading

Corentin-Allaire commented Feb 7, 2023 • edited Loading

gagnonlg commented Feb 7, 2023

Corentin-Allaire commented Feb 7, 2023

codecov bot commented Feb 7, 2023 • edited Loading

Codecov Report

github-actions bot commented Feb 7, 2023 • edited Loading

📊 Physics performance monitoring for a63f605

Vertexing

Seeding

CKF

Ambiguity resolution

Truth tracking (Kalman Filter)

Truth tracking (GSF)

benjaminhuth commented Feb 9, 2023

gagnonlg commented Feb 9, 2023

gagnonlg commented Feb 9, 2023

benjaminhuth commented Feb 9, 2023

benjaminhuth commented Feb 9, 2023 • edited Loading

gagnonlg commented Feb 9, 2023

benjaminhuth commented Feb 9, 2023

paulgessinger commented Feb 9, 2023

xju2 commented Feb 9, 2023

gagnonlg commented Feb 10, 2023

benjaminhuth commented Feb 10, 2023

gagnonlg commented Feb 10, 2023

gagnonlg commented Feb 10, 2023

paulgessinger commented Feb 7, 2023 •

edited

Loading

benjaminhuth commented Feb 7, 2023 •

edited

Loading

Corentin-Allaire commented Feb 7, 2023 •

edited

Loading

gagnonlg commented Feb 7, 2023 •

edited

Loading

gagnonlg commented Feb 7, 2023 •

edited

Loading

Corentin-Allaire commented Feb 7, 2023 •

edited

Loading

codecov bot commented Feb 7, 2023 •

edited

Loading

github-actions bot commented Feb 7, 2023 •

edited

Loading

📊 Physics performance monitoring for `a63f605`

benjaminhuth commented Feb 9, 2023 •

edited

Loading