Explain what you would like to see improved and how.
(I am sorry for opening such a "long" issue. Also, as mentioned in the disclaimer, the entire work was done by an LLM, and I am not really able to judge whether everything reported here is relevant to SOFIE or not.)
A GNN-based track finder (Belle II CAT Finder, 240 nodes, exported from PyTorch, opset 20) fails to convert with SOFIE. A patch that makes it convert, a public model that reproduces everything, and the scripts used to verify and measure are here: https://gitlab.desy.de/giacomo.depietro/catfinder-sofie
Tested on ROOT 6.41.01 @ d6fc61d0, against onnxruntime 1.29.0 (CPU EP).
The network implementation lives in basf2 at https://github.com/belle2/basf2/blob/main/tracking/scripts/gnn_tracking/__init__.py
The model has one dynamic input dimension (hits) and a neighbour-search core: four TopK operators select K = min(hits, 54) from a hits × hits distance matrix, so K is only known symbolically. That is what most of the missing support relates to.
1. Missing support — conversion aborts
ReduceMax / ReduceMin were not implemented. Added, with codegen for all three reduction paths.Elu did not accept dynamic shapes. It called GetTensorShape, which throws on a parametric dimension; now uses Dim shapes like ROperator_Relu.TopK required K to be a compile-time initializer. Now also accepts K from a shape tensor.Min/Max/Sum/Mean did not propagate shape tensors, breaking the Shape → Squeeze → Min → Reshape → Gather → Reshape chain that carries the symbolic K. ROperator_BasicBinary already had this path; the n-ary operators did not.
2. Type support
Max/Min/Mean/Sum were float-only — an INT64 Min could not be parsed. Added DOUBLE, INT32, INT64.NaryOperatorTraits<T, Mean> was specialised only for float, so the integer path would not compile. Made generic.
3. Correctness bugs — independent of this model
Reduce emits unparenthesised stride expressions. The interior-axis path generates i / 126*std::min(a,b) % (...), which parses as ((i / 126) * min) % (...). Index arithmetic is wrong and output is garbage. This also affects the already-supported ReduceMean and ReduceSum — it surfaces whenever a middle axis is reduced with shape expressions that are not single tokens, i.e. dynamic shapes.TopK ignores largest when sorted=0. That branch calls std::partial_sort with no comparator, so largest=1 returns the K smallest. The selected set is wrong, which ONNX does specify.
4. Performance — all bit-identical output
Reduce over an interior axis did two integer divisions per element. With dynamic shapes the divisors are not compile-time constants, so these are real div instructions. Replaced with one loop per axis in memory order; the inner loop is division-free and vectorises. Reduce operators went from 66% to 18% of runtime on this model.TopK used partial_sort where selection suffices — now nth_element + sort over the selected K, O(n) + O(K log K) instead of O(n log K). Safe because ties break by index, so the ordering is total and the selected set unique.TopK compared 16-byte pairs through a branching comparator. For float, (value, index) now packs into one uint64_t with an order-preserving key: one instruction per comparison, 8 bytes per element. ~1.5× on the TopK block. Non-float keeps the pair path.
Result
Generated code at -O3, both backends pinned to one thread:
hits onnxruntime SOFIE ratio
128 9.06 ms 9.18 ms 0.99x
512 53.26 ms 51.40 ms 1.04x
1024 123.91 ms 123.68 ms 1.00x
2048 269.68 ms 321.75 ms 0.84x
Related observation, not in the patch
Reshape-family copies are 12–17% of runtime. Reshape/Squeeze/Unsqueeze only reinterpret shape but each emits a std::copy (29 in this model). RModel already has most of the alternative — AddAliasTensor/IsAliasTensor, the allocator skips aliases, the liveness map extends the origin's range — and there is a commented-out call at ROperator_Slice.hxx:371. I left it alone: it touches shared infrastructure and raises questions better answered by maintainers (graph outputs need their own storage; generated code does in-place writes — ROperator_GatherND mutates its index tensor via const_cast). Happy to open a separate issue.
Reproducing
git clone https://gitlab.desy.de/giacomo.depietro/catfinder-sofie.git
cd catfinder-sofie
git -C <root-src> apply sofie-catfinder.patch # then rebuild ROOT
root -l -b -q convert_catfinder.cxx # writes catfinder.hxx and catfinder.dat
python3 benchmark_catfinder.py
python3 profile_catfinder.py --hits 512
catfinder.onnx has randomised weights — every learned parameter replaced by a draw from a standard initialisation derived from tensor shape alone, so nothing of the trained model survives. Structural constants (shapes, axes, K = 54, the exp temperature) are preserved. Outputs are meaningless; the model exists to reproduce conversion, correctness and timing.
Note both backends must be pinned to one thread: the generated code links BLAS, which otherwise oversubscribes on GEMMs far too small to pay back the threading.
Disclosure: the patch, test suites, scripts and the linked write-up were produced by Claude Opus 5 (Anthropic) from the failing conversion. Every claim is backed by a measurement in the suites above. The packed-TopK representation and the shape-tensor propagation in the n-ary operators are the two places where a maintainer's judgement on intended SOFIE semantics would be most useful.
Full details: https://claude.ai/code/artifact/f75cebba-b4fd-4e93-99e9-1880bdae2550
ROOT version
master
Installation method
Build from source
Operating system
Linux
Additional context
See https://gitlab.desy.de/giacomo.depietro/catfinder-sofie and https://claude.ai/code/artifact/f75cebba-b4fd-4e93-99e9-1880bdae2550 . The patch for ROOT is https://gitlab.desy.de/giacomo.depietro/catfinder-sofie/-/blob/main/sofie-catfinder.patch
Explain what you would like to see improved and how.
(I am sorry for opening such a "long" issue. Also, as mentioned in the disclaimer, the entire work was done by an LLM, and I am not really able to judge whether everything reported here is relevant to SOFIE or not.)
A GNN-based track finder (Belle II CAT Finder, 240 nodes, exported from PyTorch, opset 20) fails to convert with SOFIE. A patch that makes it convert, a public model that reproduces everything, and the scripts used to verify and measure are here: https://gitlab.desy.de/giacomo.depietro/catfinder-sofie
Tested on ROOT 6.41.01 @
d6fc61d0, against onnxruntime 1.29.0 (CPU EP).The network implementation lives in basf2 at https://github.com/belle2/basf2/blob/main/tracking/scripts/gnn_tracking/__init__.py
The model has one dynamic input dimension (
hits) and a neighbour-search core: fourTopKoperators selectK = min(hits, 54)from ahits × hitsdistance matrix, soKis only known symbolically. That is what most of the missing support relates to.1. Missing support — conversion aborts
ReduceMax/ReduceMinwere not implemented. Added, with codegen for all three reduction paths.Eludid not accept dynamic shapes. It calledGetTensorShape, which throws on a parametric dimension; now usesDimshapes likeROperator_Relu.TopKrequiredKto be a compile-time initializer. Now also acceptsKfrom a shape tensor.Min/Max/Sum/Meandid not propagate shape tensors, breaking theShape → Squeeze → Min → Reshape → Gather → Reshapechain that carries the symbolicK.ROperator_BasicBinaryalready had this path; the n-ary operators did not.2. Type support
Max/Min/Mean/Sumwere float-only — an INT64Mincould not be parsed. AddedDOUBLE,INT32,INT64.NaryOperatorTraits<T, Mean>was specialised only forfloat, so the integer path would not compile. Made generic.3. Correctness bugs — independent of this model
Reduceemits unparenthesised stride expressions. The interior-axis path generatesi / 126*std::min(a,b) % (...), which parses as((i / 126) * min) % (...). Index arithmetic is wrong and output is garbage. This also affects the already-supportedReduceMeanandReduceSum— it surfaces whenever a middle axis is reduced with shape expressions that are not single tokens, i.e. dynamic shapes.TopKignoreslargestwhensorted=0. That branch callsstd::partial_sortwith no comparator, solargest=1returns the K smallest. The selected set is wrong, which ONNX does specify.4. Performance — all bit-identical output
Reduceover an interior axis did two integer divisions per element. With dynamic shapes the divisors are not compile-time constants, so these are realdivinstructions. Replaced with one loop per axis in memory order; the inner loop is division-free and vectorises. Reduce operators went from 66% to 18% of runtime on this model.TopKusedpartial_sortwhere selection suffices — nownth_element+sortover the selected K, O(n) + O(K log K) instead of O(n log K). Safe because ties break by index, so the ordering is total and the selected set unique.TopKcompared 16-byte pairs through a branching comparator. For float,(value, index)now packs into oneuint64_twith an order-preserving key: one instruction per comparison, 8 bytes per element. ~1.5× on the TopK block. Non-float keeps the pair path.Result
Generated code at
-O3, both backends pinned to one thread:Related observation, not in the patch
Reshape-family copies are 12–17% of runtime.
Reshape/Squeeze/Unsqueezeonly reinterpret shape but each emits astd::copy(29 in this model).RModelalready has most of the alternative —AddAliasTensor/IsAliasTensor, the allocator skips aliases, the liveness map extends the origin's range — and there is a commented-out call atROperator_Slice.hxx:371. I left it alone: it touches shared infrastructure and raises questions better answered by maintainers (graph outputs need their own storage; generated code does in-place writes —ROperator_GatherNDmutates its index tensor viaconst_cast). Happy to open a separate issue.Reproducing
catfinder.onnxhas randomised weights — every learned parameter replaced by a draw from a standard initialisation derived from tensor shape alone, so nothing of the trained model survives. Structural constants (shapes, axes,K = 54, theexptemperature) are preserved. Outputs are meaningless; the model exists to reproduce conversion, correctness and timing.Note both backends must be pinned to one thread: the generated code links BLAS, which otherwise oversubscribes on GEMMs far too small to pay back the threading.
Disclosure: the patch, test suites, scripts and the linked write-up were produced by Claude Opus 5 (Anthropic) from the failing conversion. Every claim is backed by a measurement in the suites above. The packed-
TopKrepresentation and the shape-tensor propagation in the n-ary operators are the two places where a maintainer's judgement on intended SOFIE semantics would be most useful.Full details: https://claude.ai/code/artifact/f75cebba-b4fd-4e93-99e9-1880bdae2550
ROOT version
master
Installation method
Build from source
Operating system
Linux
Additional context
See https://gitlab.desy.de/giacomo.depietro/catfinder-sofie and https://claude.ai/code/artifact/f75cebba-b4fd-4e93-99e9-1880bdae2550 . The patch for ROOT is https://gitlab.desy.de/giacomo.depietro/catfinder-sofie/-/blob/main/sofie-catfinder.patch