Skip to content

[tmva][sofie] Remove Keras and PyTorch parses from ROOT repository - #22734

Merged
guitargeek merged 1 commit into
root-project:masterfrom
guitargeek:sofie_keras_torch
Aug 21, 2026
Merged

[tmva][sofie] Remove Keras and PyTorch parses from ROOT repository#22734
guitargeek merged 1 commit into
root-project:masterfrom
guitargeek:sofie_keras_torch

Conversation

@guitargeek

@guitargeek guitargeek commented Jun 30, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

As announced in the Q2 2026 Report and Planning meeting [1], the SOFIE distribution within ROOT will be slimmed down to keep only the core functionality of enabling ONNX model inference on the CPU with ROOT via ONNX parsing and C++ code emit.

That means to keep the ONNX parser, and to remove the Keras and PyTorch parsers. these parsers relied on private implementation details of the Keras or PyTorch libraries, which change faster than appropriate for the stability standards of ROOT. We expect users to choose neural net architectures that are serializable by ONNX for production use.

The Keras and PyTorch parsers remain valuable work, visible in ROOTs commit history and ready to be re-distributed by other parties, which would be very easy since this is just Python code. It would be trivial to ship this as a Python package that requires ROOT or another distribution of C++ SOFIE.

[1] https://indico.cern.ch/event/1699702/ : slide 19

A demo for how it could be packaged outside of ROOT is given in: https://github.com/guitargeek/sofie-extra-parsers

@guitargeek guitargeek self-assigned this Jun 30, 2026
@guitargeek guitargeek added clean build Ask CI to do non-incremental build on PR in:SOFIE labels Jun 30, 2026
@github-actions

github-actions Bot commented Jun 30, 2026 •
edited
Loading

Copy link
Copy Markdown

Test Results

    23 files      23 suites   3d 12h 26m 51s ⏱️
 3 851 tests  3 850 ✅ 0 💤 1 ❌
78 494 runs  78 492 ✅ 1 💤 1 ❌

For more details on these failures, see this check.

Results for commit 64a153f.

♻️ This comment has been updated with latest results.

@guitargeek
guitargeek force-pushed the sofie_keras_torch branch 3 times, most recently from aff6bf7 to b5be88e Compare July 13, 2026 06:49
@guitargeek
guitargeek force-pushed the sofie_keras_torch branch 3 times, most recently from cb23ac9 to b44f619 Compare July 24, 2026 16:30
@guitargeek
guitargeek marked this pull request as ready for review July 24, 2026 16:30
dpiparo
dpiparo approved these changes Jul 28, 2026
View reviewed changes

@dpiparo dpiparo left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

If lorenzo and sanjiban agree, LGTM.

@guitargeek guitargeek mentioned this pull request Aug 9, 2026
Merged
@guitargeek
[tmva][sofie] Remove Keras and PyTorch parses from ROOT repository
64a153f 
As announced in the Q2 2026 Report and Planning meeting [1], the SOFIE
distribution within ROOT will be slimmed down to keep only the core
functionality of enabling ONNX model inference on the CPU with ROOT via
ONNX parsing and C++ code emit.

That means to keep the ONNX parser, and to remove the Keras and PyTorch
parsers. these parsers relied on private implementation details of the
Keras or PyTorch libraries, which change faster than appropriate for the
stability standards of ROOT. We expect users to choose neural net
architectures that are serializable by ONNX for production use.

The Keras and PyTorch parsers remain valuable work, visible in ROOTs
commit history and ready to be re-distributed by other parties, which
would be very easy since this is just Python code. It would be trivial
to ship this as a Python package that requires ROOT or another
distribution of C++ SOFIE.

[1] https://indico.cern.ch/event/1699702/ : slide 19
This was referenced Aug 19, 2026
Closed
Closed
Pranav-error added a commit to Pranav-error/root that referenced this pull request Aug 20, 2026
@Pranav-error
[tmva][sofie] Drop Keras and PyTorch parser references from the README
fe86197 
Following review feedback: rather than repointing the links, remove the mentions
altogether, since both parsers are being removed in root-project#22734 in favour of going
through ONNX export.

- the parser-generator paragraph now names only the ONNX parser
- the TMVA_SOFIE_Keras, TMVA_SOFIE_PyTorch and TMVA_SOFIE_PyTorch_HiggsModel
  tutorial links are dropped
Pranav-error added a commit to Pranav-error/scientific-ml-pipelines that referenced this pull request Aug 20, 2026
@Pranav-error @claude
Stop the SOFIE probe from claiming a measurement it never made
9ba76d6 
The README said the probe "measures that gap rather than asserting it". It does not. The
conda-forge ROOT package does not enable SOFIE at all — root-config --features on 6.40.02
lists tmva, tmva-cpu, tmva-cudnn and tmva-pymva with no tmva-sofie, and
TMVA::Experimental::SOFIE exposes PyKeras but neither PyTorch nor RModelParser_ONNX. So
the probe could never have run in the only environment documented for it, and the operator
counts in the README are read from ROOT's source rather than observed.

Both probes now check that precondition first and exit naming the missing feature, so an
environment that cannot measure anything can no longer be mistaken for a passing run. The
README states plainly that the numbers are source-derived and what a real measurement
would require.

Add probe_onnx_parser.py for the PyTorch -> ONNX -> SOFIE route, separating ONNX export,
parse and C++ codegen failures. That route is the one that matters going forward:
root-project/root#22734 proposes removing the Keras and PyTorch parsers and keeping only
ONNX, which makes the original probe's target a historical question. It is written and
guarded but, for the same reason, unmeasured.

Also fix artefact cleanup in the ONNX probe, which left exported .onnx files behind on
every failure path.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Pranav-error added a commit to Pranav-error/scientific-ml-pipelines that referenced this pull request Aug 20, 2026
@Pranav-error @claude
Measure the SOFIE ONNX route for the first time; PyTorch route stays …
ce6a93e 
…unmeasured

Built ROOT 6.40.02 from source with -Dtmva-sofie=ON, since conda-forge ships without it.

ONNX route: 11 of 11 operators pass parse and C++ codegen at opset 13, including pooling,
batch normalisation, residual add, softmax, tanh, leaky relu, flatten and concat. That is
the route that survives root-project/root#22734, which removes the Keras and PyTorch
parsers and has already landed on master. The path that remains carries everything the
removed one could not.

PyTorch route: reports 0 of 11 on this build, and that number is NOT claimed as a result.
Gemm and Relu are controls the parser documentably supports and they fail identically,
with cling JIT errors about a libc++ ABI mismatch and a std_darwin.modulemap missing
__configuration/experimental.h. A failing control means the instrument is measuring
itself, so the 6-of-56 figure stays labelled as read from ROOT's source.

Also record why the build needs -Dtmva-pymva=ON: ROOT 6.40 requests only
Development.Module when pymva is off, yielding Python3::Module, while sofie_parsers links
ROOTTMVASofiePyParsers against Python3::Python unconditionally. Fixed on master.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Pranav-error added a commit to Pranav-error/scientific-ml-pipelines that referenced this pull request Aug 20, 2026
@Pranav-error @claude
Measure both SOFIE routes; the PyTorch parser reaches 3 of 11, not th…
628cb42 
…e documented 6

First real numbers from either probe, on a source-built ROOT 6.40.02 with -Dtmva-sofie=ON.

  PyTorch parser   3/11   Gemm, Relu, BatchNormalization
  via ONNX        11/11   everything, parse and codegen

Measuring corrected the claim the probe was built to check. Reading ROOT's source says the
PyTorch parser maps six operators — Gemm, Conv, Relu, Selu, Sigmoid, Transpose — and that
pooling, batch normalisation and residual connections are therefore unreachable. Batch
normalisation parses. It is absent from that list, so the list was not the whole story and
the README repeated its conclusion as fact. The other eight failures are genuine and each
reports "Parsing PyTorch node onnx::X is not yet supported".

Every operator the outgoing parser cannot reach survives the ONNX route, which is the one
that matters: root-project/root#22734 removes the Keras and PyTorch parsers and has already
landed on master.

Getting a valid number needed the probe rewritten to run as a ROOT macro. Driven through
PyROOT the parser fails on every case including the controls, because its embedded
interpreter cannot "import torch"; the identical model parses from a .C macro. Both probes
now treat Gemm and Relu as controls and refuse to report anything if they fail, which is
what stopped that harness bug — and the unSOFIE'd conda build before it — from being
published as "0 of 11 operators supported".

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@guitargeek

guitargeek commented Aug 21, 2026

Copy link
Copy Markdown
Contributor Author

Merging after discussions with both @dpiparo and @lmoneta. Lorenzo would have preferred to merge this after the standalone version is integrated in the ML4EP repositories, but given that we want to merge this PR in the current dev cycle anyway, it doesn't really matter if we do it now or later. This code is in the history and 6.40 branch anyway. And removing is as soon as possible has the benefit that we don't risk misleading external contributors, who want to help extending operator coverage for the parsers that we intend to remove anyway.

@guitargeek
guitargeek merged commit b881850 into root-project:master Aug 21, 2026
31 of 34 checks passed
@guitargeek
guitargeek deleted the sofie_keras_torch branch August 21, 2026 14:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@dpiparo dpiparo dpiparo approved these changes
@bellenot bellenot Awaiting requested review from bellenot bellenot is a code owner
@couet couet Awaiting requested review from couet
@lmoneta lmoneta Awaiting requested review from lmoneta lmoneta is a code owner
@vepadulano vepadulano Awaiting requested review from vepadulano vepadulano is a code owner

Assignees

@guitargeek guitargeek

Labels

clean build Ask CI to do non-incremental build on PR in:SOFIE

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@guitargeek @dpiparo