Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[cross check]: updating and merging dev-align3 #429

Closed
wants to merge 364 commits into from
Closed

Conversation

c-dilks
Copy link
Member

@c-dilks c-dilks commented Jan 13, 2025

This should be very similar to #425, since it was made independently with the help of @sebouh137 and Sebastian to resolve the merge conflicts (~Feb. 2024)

Because @raffaelladevita independently resolved merge conflicts in #425, we can compare these two PRs.

sebouh137 and others added 30 commits May 18, 2021 12:32
allows pre-alignment to obtain some of the information directly from the recon, instead of recalculating a lot of information
* using https for freehep

* one more attempt

* trying via mirror

* trying via mirror

* cleanup

Co-authored-by: Nathan Baltzell <[email protected]>
Lorentz angle correction fix.
Fix in BMT uncertainty used in the fit.
Fix in using isolated SVT clusters in track refit.
Hit sorting using time and Edep.
New clustering implemented; Default = off.
sebouh137 and others added 21 commits January 31, 2022 06:34
…nt. (using curved tracks in alignment still is not functional yet.)
… use existing RecoBankReader class, fixed issues with line-cylinder intersection in straight track KF transport
@c-dilks
Copy link
Member Author

c-dilks commented Jan 13, 2025

@raffaelladevita, here are the diffs:

Also I'm getting build errors in the CI (whereas #425 passes CI checks)

@raffaelladevita
Copy link
Collaborator

I checked the differences, comparing manually the two branches since git is showing differences that are not really there. The relevant modifications are:

All other differences seems to be different coding choices

@c-dilks
Copy link
Member Author

c-dilks commented Jan 14, 2025

fixed in 3 recent commits

  • Also, the variables xb, yb should be read from the bank and not set to 0.

looks like this will take a bit more effort... perhaps we just use your version of AlignmentBankReader?

@c-dilks
Copy link
Member Author

c-dilks commented Jan 16, 2025

discussed with @raffaelladevita, there are no more significant differences, aside from necessary changes in

reconstruction/cvt/src/main/java/org/jlab/rec/cvt/alignment/AlignmentBankReader.java
reconstruction/cvt/src/main/java/org/jlab/rec/cvt/banks/RecoBankReader.java

our backmerge (development -> this branch) conflict resolutions are consistent.

closing this PR; proceed with testing #425

@c-dilks c-dilks closed this Jan 16, 2025
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.

6 participants