Skip to content

feat: remove unused nodes before each step in GNN workflow#5448

Open
benjaminhuth wants to merge 14 commits into
acts-project:mainfrom
benjaminhuth:feature/gnn-remove-unused-nodes
Open

feat: remove unused nodes before each step in GNN workflow#5448
benjaminhuth wants to merge 14 commits into
acts-project:mainfrom
benjaminhuth:feature/gnn-remove-unused-nodes

Conversation

@benjaminhuth

Copy link
Copy Markdown
Member

No description provided.

@benjaminhuth benjaminhuth added this to the next milestone May 13, 2026
@github-actions github-actions Bot added Component - Examples Affects the Examples module Component - Plugins Affects one or more Plugins Track Finding labels May 13, 2026
@benjaminhuth benjaminhuth force-pushed the feature/gnn-remove-unused-nodes branch from 6150a21 to b14d556 Compare May 15, 2026 10:14
@benjaminhuth benjaminhuth marked this pull request as ready for review May 18, 2026 07:50
@github-actions

github-actions Bot commented May 18, 2026

Copy link
Copy Markdown
Contributor

📊: Physics performance monitoring for 637c60e

Full contents

physmon summary

benjaminhuth and others added 8 commits May 20, 2026 10:41
…ependency

The torchscript gnn.pt model uses torch_scatter::scatter_max which is not
available in CI. Switch to onnx_models/gnn.onnx, consistent with the fix
already applied in tmp/gnn-remove-unused-nodes-with-main.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
mimodekjaer
mimodekjaer previously approved these changes Jun 10, 2026

@mimodekjaer mimodekjaer left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Seems okay.

The only real comment is that the clang formatting should be fixed. :)

The metric learning workflow uses torch_scatter which is not available in CI.
Switched test_gnn_shrink_nodes_same_output to the ODD GNN module map workflow,
which uses the ONNX model already proven to work in CI.

Added shrinkNodes parameter to runGnnModuleMap and pass it through to addGnn.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@acts-policybot acts-policybot Bot dismissed mimodekjaer’s stale review June 15, 2026 09:09

Invalidated by push of d42eefa

Switch from .onnx to .pt model (ONNX CUDA provider not available in
all environments) and explicitly delete the sequencer after each run so
the ROOT TFile destructor flushes before hash_root_file reads it.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@benjaminhuth benjaminhuth requested a review from mimodekjaer June 15, 2026 14:02
@sonarqubecloud

Copy link
Copy Markdown

@mimodekjaer mimodekjaer left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

I think all in all, that it is good :), but I think it would be good to check that the root files are generated, before accessing them.

Comment thread Python/Examples/tests/test_examples.py
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

automerge Component - Examples Affects the Examples module Component - Plugins Affects one or more Plugins Track Finding

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants