Eventbuilder by sophiemiddleton · Pull Request #2029 · LDMX-Software/ldmx-sw

sophiemiddleton · 2026-04-19T21:30:25Z

I am updating ldmx-sw, here are the details.

What are the issues that this addresses?

Check List

I successfully compiled ldmx-sw with my developments.
I read, understood and follow the coding rules.

I ran my developments and the following shows that they are successful.

also patched eventbuilder python configuration module to load the appropriate C++ library

tvami · 2026-04-20T14:20:30Z

Is this ready for the review Sophie?

It does not compile, with

/home/runner/work/ldmx-sw/ldmx-sw/EventBuilder/src/EventBuilder/EventBuilder.cxx: In member function 'virtual void eventbuilder::EventBuilder::produce(framework::Event&)':
/home/runner/work/ldmx-sw/ldmx-sw/EventBuilder/src/EventBuilder/EventBuilder.cxx:227:71: error: 'getSubsystemName' is not a member of 'packing::LDMXRoRHeader'
  227 |                     std::string subsys_name = packing::LDMXRoRHeader::getSubsystemName(
      |                                                                       ^~~~~~~~~~~~~~~~
/home/runner/work/ldmx-sw/ldmx-sw/EventBuilder/src/EventBuilder/EventBuilder.cxx:334:63: error: 'getSubsystemName' is not a member of 'packing::LDMXRoRHeader'
  334 |             std::string subsys_name = packing::LDMXRoRHeader::getSubsystemName(
      |                                                               ^~~~~~~~~~~~~~~~

sophiemiddleton · 2026-04-20T16:40:59Z

Let me check, perhaps I didnt push all the code (there was an issue with my branch due to a failed attempt to push a large data file).

sophiemiddleton · 2026-04-20T16:41:40Z

its likely I just didnt add the Packing package from that printout, so perhaps its an easy fix..

tvami · 2026-04-20T21:11:02Z

/run-validation

github-actions · 2026-04-20T21:11:10Z

The validation workflow is running here: https://github.com/LDMX-Software/ldmx-sw/actions/runs/24690604188.

tvami

Thanks Sophie for this work! I've made a handful cosmetic / technical comments. It would be good to have the input you used to do some real testing. I'd likely also need to have a second read as there is quite some casting ongoing that I'm not getting yet

tvami · 2026-04-20T21:19:58Z

+from LDMX.EventBuilder import eventbuilder
+import sys
+p.sequence = [
+    eventbuilder.from_dat_file(sys.argv[1])


How big is the input you used to test it? Wonder if we can upload it to the ci-data repo

It can be, it was too large to push, i know that but we could use it for validation still

This is the repo I had in mind: https://github.com/LDMX-Software/ci-data

yes, we can put something there

what's the best way to add the .dat here. I'm guessing manual upload?

Yeah, but what's the size? If it's too big we could consider skimming to less events

tvami · 2026-04-20T21:22:51Z

+        super().__init__(instance_name, 'eventbuilder::EventBuilder', 'EventBuilder')
+        self.dat_file = dat_file or ''
+        self.output_name = output_name
+        self.verbose_parse = verbose_parse


This is not the current way to have classes, please have a look at any class now in trunk

I think this requires a full update of the software stack (?) . Tom mentioned it. I think it might be better to do that once this is merged. Since that was my plan. Unless its more seamless than I'm assuming?

I'm not sure what you mean by "full update of the software stack". I think you can (and should) update the implementation of this class to be in sync with the other stuff on trunk before it is merged. I am willing to help you do this if you are unsure on what to do from looking at the other python config classes on trunk.

I just meant, if I change my syntax alone then it will probably not be correct. I need to pull the latest code or latest environment or something too (instructions might be useful, so I dont become incompatible)

Ah sure, here is my recipe for updating to be in sync with trunk. I don't think there will be issues because I tried it locally and it worked and the CI is compiling fine and the CI uses the newest environment.

# 1. download trunk from GitHub to make sure its up to date git switch trunk git pull origin trunk # 2. merge the trunk branch with yours git switch eventbuilder git merge trunk # 3. there was an update to the SimCore/G4DarkBreM submodule so you'll need to also run git submodule update

As for updating to the latest environment, you can just pull ldmx/dev:latest from within your clone of ldmx-sw.

Personally, I prefer to use git rebase instead of git merge, but the difference between those two is lost after we squash-merge the PR into trunk anyways.

OK, let me try this later today - will let you know if I need more information

tvami · 2026-04-20T21:47:53Z

How do you interpret
https://private-user-images.githubusercontent.com/4170148/580488937-f87b01af-6d35-487b-8e2c-aa1912b519da.png?jwt=eyJ0eXAiOiJKV1QiLCJhbGciOiJIUzI1NiJ9.eyJpc3MiOiJnaXRodWIuY29tIiwiYXVkIjoicmF3LmdpdGh1YnVzZXJjb250ZW50LmNvbSIsImtleSI6ImtleTUiLCJleHAiOjE3NzY3MjE3MzEsIm5iZiI6MTc3NjcyMTQzMSwicGF0aCI6Ii80MTcwMTQ4LzU4MDQ4ODkzNy1mODdiMDFhZi02ZDM1LTQ4N2ItOGUyYy1hYTE5MTJiNTE5ZGEucG5nP1gtQW16LUFsZ29yaXRobT1BV1M0LUhNQUMtU0hBMjU2JlgtQW16LUNyZWRlbnRpYWw9QUtJQVZDT0RZTFNBNTNQUUs0WkElMkYyMDI2MDQyMCUyRnVzLWVhc3QtMSUyRnMzJTJGYXdzNF9yZXF1ZXN0JlgtQW16LURhdGU9MjAyNjA0MjBUMjE0MzUxWiZYLUFtei1FeHBpcmVzPTMwMCZYLUFtei1TaWduYXR1cmU9MDQ0OTg5ZGY1NGJlZjRiOTkxZjVlZTk1ZDNmOTZjOGE4ZTE0OTIxYmU3ZjIyNGU4YjlhMDY2OWZkYzg5NzMzMiZYLUFtei1TaWduZWRIZWFkZXJzPWhvc3QmcmVzcG9uc2UtY29udGVudC10eXBlPWltYWdlJTJGcG5nIn0.aFudbFNpLdRx7uiLDtol4Aoq8pfn2b_V6r1U4CSArU4

btw? What's all the zeros? And the periodic bumps?

github-actions · 2026-04-20T23:32:09Z

Validation Results

Some validation samples failed! ❌

Sample	Status
`deep_ecal_gun`	✅ PASS
`eat_signal`	✅ PASS
`ecal_pn`	✅ PASS
`hcal`	✅ PASS
`inclusive`	✅ PASS
`it_pileup`	✅ PASS
`reduced_ldmx`	✅ PASS
`signal`	✅ PASS
`signal_target_al`	✅ PASS
`target_pn_lyso`	✅ PASS
`target_ti_en`	✅ PASS
`wab_lhe`	✅ PASS
`kaon_enhanced`	❌ FAIL (1105 histograms failed KS test)
`target_genie`	❌ FAIL (11 histograms failed KS test)

sophiemiddleton · 2026-04-21T01:28:46Z

Thanks, ill look at theae tomorrow. The cose has evolved through many iterations and in three different environments hence the mismatches in ldmx style. I will correct the few points and push

sophiemiddleton · 2026-04-21T01:29:52Z

With regards to the plot. I checked using just a subsystem packer of Toms and the shape was the same. I need to decode for it to make any sense really. Thats the next step....

Co-authored-by: Tamas Vami <tamas.almos.vami@cern.ch>

github-actions · 2026-04-21T01:30:35Z

Validation Results

Some validation samples failed! ❌

Sample	Status
`deep_ecal_gun`	✅ PASS
`eat_signal`	✅ PASS
`ecal_pn`	✅ PASS
`hcal`	✅ PASS
`inclusive`	✅ PASS
`it_pileup`	✅ PASS
`reduced_ldmx`	✅ PASS
`signal`	✅ PASS
`signal_target_al`	✅ PASS
`target_genie`	✅ PASS
`target_pn_lyso`	✅ PASS
`target_ti_en`	✅ PASS
`wab_lhe`	✅ PASS
`kaon_enhanced`	❌ FAIL (1105 histograms failed KS test)

Co-authored-by: Tamas Vami <tamas.almos.vami@cern.ch>

sophiemiddleton · 2026-04-21T14:15:03Z

I have mostly completed the requests above (minus a few). For your casting doubts, I will try to improve things to remove implicit casting. Hopefully that helps alleviate some of the concerns.

sophiemiddleton · 2026-04-27T14:01:48Z

@tvami - was there anything else you needed me to fix here?

My next steps are to work with detector teams to decode the events and make plots to see if they make sense (this links to what was discussed on Wednesday)

tvami · 2026-04-27T15:45:21Z

I was hoping somebody else (@tomeichlersmith maybe?) would also chime in with the review before I have a second look. You finished with the casting points, right?

tomeichlersmith

I have two larger thoughts and I'm choosing to not delve much deeper. Partially due to laziness, but also because I don't want to get stuck in my own perfectionist mindset. I think the better solution would be merge this (as long as it compiles on trunk) and then we can start iteratively developing it as folks use it and we test it more thoroughly.

tomeichlersmith · 2026-04-27T16:42:25Z

+        super().__init__(instance_name, 'eventbuilder::EventBuilder', 'EventBuilder')
+        self.dat_file = dat_file or ''
+        self.output_name = output_name
+        self.verbose_parse = verbose_parse


I'm not sure what you mean by "full update of the software stack". I think you can (and should) update the implementation of this class to be in sync with the other stuff on trunk before it is merged. I am willing to help you do this if you are unsure on what to do from looking at the other python config classes on trunk.

tvami · 2026-05-14T18:20:25Z

@sophiemiddleton do you need help with this?

sophiemiddleton · 2026-05-14T20:13:22Z

No, I just got busy in the past few weeks. I'll put some time aside to try to get this done in the coming week

sophiemiddleton and others added 19 commits February 18, 2026 07:36

EB

75d682d

EB update

392debc

EB update

7d53de7

moved dirs

37311e8

removed duplicates

101406c

tidy EventBuilder CMakeLists.txt out of habit

670920d

need to include EventBuilder subdirectory in central CMakeLists.txt

dbceeff

two example configs (SingleSubsystemUnpacker and EventBuilder)

a5fcc8d

also patched eventbuilder python configuration module to load the appropriate C++ library

working builder

d361d0e

working builder

10b41ae

working builder

4092d0d

working builder

4d428f3

working builder

d800390

finalized summary product, added DAQ diagnostics

4a35a30

finalized summary product, added DAQ diagnostics

28badfd

updated

0ad54f3

updated

3c243c8

removed depracated class

9318f10

edits for contributor id

f183f12

copied Packing over

f80da6f

tvami requested changes Apr 20, 2026

View reviewed changes

Update EventBuilder/include/EventBuilder/FragmentBuffer.hh

e885362

Co-authored-by: Tamas Vami <tamas.almos.vami@cern.ch>

Update EventBuilder/include/EventBuilder/FragmentBuffer.hh

73c4daa

Co-authored-by: Tamas Vami <tamas.almos.vami@cern.ch>

sophiemiddleton added 3 commits April 21, 2026 07:42

Tamas suggestions

f61d59a

Tamas suggestions

d8958aa

Tamas suggestions

c590709

tomeichlersmith mentioned this pull request Apr 21, 2026

unpack multiple subsystems into LDMX Events from Rogue data files #1930

Open

tomeichlersmith requested changes Apr 27, 2026

View reviewed changes

Conversation

sophiemiddleton commented Apr 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What are the issues that this addresses?

Check List

Uh oh!

tvami commented Apr 20, 2026

Uh oh!

sophiemiddleton commented Apr 20, 2026

Uh oh!

sophiemiddleton commented Apr 20, 2026

Uh oh!

tvami commented Apr 20, 2026

Uh oh!

github-actions Bot commented Apr 20, 2026

Uh oh!

tvami left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

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!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

tvami commented Apr 20, 2026

Uh oh!

github-actions Bot commented Apr 20, 2026

Validation Results

Uh oh!

sophiemiddleton commented Apr 21, 2026

Uh oh!

sophiemiddleton commented Apr 21, 2026

Uh oh!

github-actions Bot commented Apr 21, 2026

Validation Results

Uh oh!

sophiemiddleton commented Apr 21, 2026

Uh oh!

sophiemiddleton commented Apr 27, 2026

Uh oh!

tvami commented Apr 27, 2026

Uh oh!

tomeichlersmith left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tvami commented May 14, 2026

Uh oh!

sophiemiddleton commented May 14, 2026

Uh oh!

Reviewers

sophiemiddleton commented Apr 19, 2026 •

edited

Loading