Skip to content

Adds ParameterHit info to KalSeed so it can be regrown#1853

Open
bonventre wants to merge 3 commits into
Mu2e:mainfrom
bonventre:paramhit
Open

Adds ParameterHit info to KalSeed so it can be regrown#1853
bonventre wants to merge 3 commits into
Mu2e:mainfrom
bonventre:paramhit

Conversation

@bonventre

Copy link
Copy Markdown
Contributor

KKLine uses parameterhit to constrain momentum etc., plus we expect to add constraints for reflecting cosmics etc. This adds the info to the persistent object so we can regrow it

@bonventre bonventre requested a review from brownd1978 June 8, 2026 18:17
@FNALbuild

Copy link
Copy Markdown
Collaborator

Hi @bonventre,
You have proposed changes to files in these packages:

  • CosmicReco
  • Mu2eKinKal
  • RecoDataProducts

which require these tests: build.

@Mu2e/fnalbuild-users, @Mu2e/write have access to CI actions on main.

⌛ The following tests have been triggered for 0dbe116: build (Build queue - API unavailable)

About FNALbuild. Code review on Mu2e/Offline.

@FNALbuild

Copy link
Copy Markdown
Collaborator

☀️ The build tests passed at 0dbe116.

Test Result Details
test with Command did not list any other PRs to include
merge Merged 0dbe116 at d755001
build (prof) Log file. Build time: 16 min 24 sec
ceSimReco Log file.
g4test_03MT Log file.
transportOnly Log file.
POT Log file.
g4study Log file.
cosmicSimReco Log file.
cosmicOffSpill Log file.
ceSteps Log file.
ceDigi Log file.
muDauSteps Log file.
ceMix Log file.
rootOverlaps Log file.
g4surfaceCheck Log file.
trigger Log file.
check_cmake Log file.
FIXME, TODO ➡️ TODO (21) FIXME (5) in 10 files
clang-tidy ➡️ 12 errors 153 warnings
whitespace check no whitespace errors found

N.B. These results were obtained from a build of this Pull Request at 0dbe116 after being merged into the base branch at d755001.

For more information, please check the job page here.
Build artifacts are deleted after 5 days. If this is not desired, select Keep this build forever on the job page.

@brownd1978 brownd1978 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Thanks for filling in this missing piece to regrowing. I'm probably missing something, but I don't understand why the extendTrack method needs parameter hits. I'd also like to see the code duplcation removed if possible.

Outside the scope of this PR; some codes will need updating to handle the reflection constraint, but I don't see anything here that breaks that. That constraint is based on ParameterConstraint but also has payload for the coupling between the tracks. I believe the persistent form can either inherit from or embed TrkParamHitSeed.

}
// set the seed range given the hit TPOCA values
seedtraj.range() = kkfit_.range(strawhits,calohits, strawxings);

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

This code block is identical to the one in KinematicLineFit, and might be useful in LoopHelix as well. Can it be moved to KKFit or some other shared utility?

Comment thread Mu2eKinKal/inc/KKFit.hh Outdated
<< addstrawxings.size() << " Straw Xings" << std::endl;
}
kktrk.extendTrack(exconfig,addstrawhits,addstrawxings,addcalohits);
kktrk.extendTrack(exconfig,addstrawhits,addstrawxings,addcalohits,addparamhits);

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I don't understand why parameters hits are needed in the extendTrack interface. The original intent of having this call here was to allow additional schedule steps beyond simply reconstituting a regrown fit, perhaps to add new hits, or regrowing a seed fit to a drift fit, etc. I don't see how new parameter hits could be added in that context. In this call the parameter hits vector is hard-coded to empty, what is the use case for a call with a non-zero vector? I don't see such a call to it in this PR.

@brownd1978 brownd1978 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Thanks for making the requested changes, all looks good now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants