Skip to content

Rebase of muon collider fork - #50

Closed
aloeliger wants to merge 14 commits into
key4hep:mainfrom
aloeliger:k4h_rebase
Closed

Rebase of muon collider fork#50
aloeliger wants to merge 14 commits into
key4hep:mainfrom
aloeliger:k4h_rebase

Conversation

@aloeliger

Copy link
Copy Markdown
Contributor

This PR represents a rebase of developments made in the muon collider fork as part of the push to synchronize these developments with the main key4hep repository. This is largely being opened for discussion to start the process of reconciling these two forks, if the ultimate PR requires a selected subset of these changes, that is something that can be discussed.

@madbaron @tmadlener @samf25 FYI

BEGINRELEASENOTES

  • Synchronization of muon collider developments
  • k4reco addition of fast jet
  • update of podio and k4geo requirements
  • Addition of calorimeter hits
  • Addition of OverlayTimingRandomMix
    ENDRELEASENOTES

@jmcarcell

Copy link
Copy Markdown
Member

This is way too big to review. Can it be split in independent set of changes? I see several parts: overlay, tracking, fastjet, etc. And sometimes it's changes to existing files like the GaudiDDKalTest ones, others it's adding new files, those could go in separate PRs.

@samf25

samf25 commented Mar 9, 2026

Copy link
Copy Markdown
Contributor

Yes, I think it makes sense to start discussing this. I was wondering if it might even make sense to split up this repo.

I know that historically (in the Marlin version) a lot of these were packaged together, but, for instance, DDPlanarDigi, CaloDigi, and the Overlay algorithms are digitization level algorithms (so it might be better for them to live somewhere like k4Digi)

Additionally, does it make sense to group the tracking reconstruction algorithms with the calorimetry ones? I.e. ConformalTracking, Tracking, and GaudiTrkUtils makes sense. While GaudiLumiCalClusterer and FastJet might be different.

Then there's also k4ActsTracking which is definitely k4Reco and k4GaudiPandora (where I think there has also been a bit of divergence between muon collider and key4hep upstream)

Just my thoughts, so take them or leave them! I just fear that this repo will quickly grow large and unweildy.

@aloeliger

Copy link
Copy Markdown
Contributor Author

Hi @samf25. Apologies it is weird meeting like this, but I have been talking with @tmadlener and @madbaron (to start helping out with muon collider software) about starting rebases of those very packages as well. k4ACTSTracking is trivial, and can be deprecated on the muon collider side right now (it is behind with no unique commits). k4GaudiPandora has quite significant divergence between muon collider forks and this one and is not as simple a rebase as this might be.

To @jmcarcell's point, perhaps you and I should discuss at some point (you were the primary figure behind most of these changes going by the history) the changes made, and how they might be made more modular for pushing upstream.

@samf25

samf25 commented Mar 9, 2026

Copy link
Copy Markdown
Contributor

Yeah, that would make sense to me -- happy to meet to chat and work out a modular strategy. We can move our chat to gmail or some such and schedule a time?

@tmadlener

Copy link
Copy Markdown
Member

I think there are a lot valid points here that warrant discussion. That was one of the reasons to get this PR started as it can serve as an anchor for this discussion. I have no really strong feelings in regards to splitting this repo up into smaller more topical ones or making it more like a "one stop shop" for (almost) everything after reconstruction. There are pros and cons in both directions. We will have to discuss that on the Key4hep level, I guess.

It also looks like some of the things that ended up here for the muon collider, went to k4GaudiPandora for the upstream Key4hep, e.g. https://github.com/key4hep/k4GaudiPandora/blob/main/k4GaudiPandora/include/CalorimeterHitType.h (and it's implementation). Is it used here from this PR?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

What is different in this algorithm compared to OverlayTiming?

@madbaron

Copy link
Copy Markdown
Member

I think this PR can be closed as superseeded:

@madbaron madbaron closed this Aug 29, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants