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

Inner mpgd barrel geo update #732

Merged
merged 29 commits into from
Jul 30, 2024
Merged

Inner mpgd barrel geo update #732

merged 29 commits into from
Jul 30, 2024

Conversation

ybedfer
Copy link
Contributor

@ybedfer ybedfer commented May 21, 2024

Briefly, what does this PR introduce?

Update the MPGD inner barrel, making it a set of cylindrical, as opposed to flat, tiles.

What kind of change does this PR introduce?

Please check if this PR fulfills the following:

  • Changes have been communicated to collaborators

Does this PR introduce breaking changes? What changes might users need to make to their code?

Yes. Users need to update their DD4hep private repository to revision #c3b305ce of the AIDASoft/DD4hep github, see "AIDASoft/DD4hep@c3b305ce".

Does this PR change default behavior?

Yes.

@ybedfer ybedfer requested a review from mposik1983 May 21, 2024 14:43
@ybedfer ybedfer self-assigned this May 21, 2024
@github-actions github-actions bot added topic: tracking topic: forward Positive-rapidity detectors (hadron-going side) labels May 21, 2024
@github-actions github-actions bot added the topic: barrel Mid-rapidity detectors label Jun 28, 2024
@ybedfer ybedfer requested a review from wdconinc July 24, 2024 09:57
@ybedfer
Copy link
Contributor Author

ybedfer commented Jul 24, 2024

Plots showing the distributions of:

  • Simulated Hits,
  • Reconstructed Hits,
  • Residuals (Rec-Sim).

Setup = ddsim with a gun of µ^- in the range 10 GeV, 15 < theta < 165 deg.

cCyMBaLSim.pdf
cCyMBaLRec.pdf
cCyMBaLRes.pdf

Remarks:

  • The different colors correspond to sub-structures: red/blue = tiles along φ, else 4 sectors along Z; black is the overall.
  • On the Simulated plot, some stray hits show up in the Rr (radius) distribution. This is non physical, but seems to be a common feature also present in the Outer MPGD detector, hence not connected to this PR.
  • When going from Sim to Rec, there's some inefficiency. Feature also seen in all barrel trackers.
  • Std Dev of residuals are as expected for a 150 $\times\sqrt{12}$ µm grid size in Z and 1~= $\sqrt{12}\times$.0.29 mrd in φ.

eicrecon

Executed on the ddsim data mentioned supra, with debugging printout ( -Ptracking:CentralTrackerMeasurements:LogLevel=trace), hits from CyMBaL are seen to be associated to tracks.

@wdconinc
Copy link
Contributor

Copy link
Contributor

@wdconinc wdconinc left a comment

Choose a reason for hiding this comment

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

This looks good. Thanks! I added some comments. I will wait to approve until later this when the pipelines should be able to run.

compact/tracking/mpgd_barrel.xml Outdated Show resolved Hide resolved
compact/tracking/mpgd_barrel.xml Outdated Show resolved Hide resolved
compact/tracking/mpgd_barrel.xml Show resolved Hide resolved
compact/tracking/mpgd_barrel.xml Show resolved Hide resolved
src/MPGDCylinderBarrelTracker_geo.cpp Outdated Show resolved Hide resolved
src/MPGDCylinderBarrelTracker_geo.cpp Outdated Show resolved Hide resolved
src/MPGDCylinderBarrelTracker_geo.cpp Outdated Show resolved Hide resolved
src/MPGDCylinderBarrelTracker_geo.cpp Show resolved Hide resolved
src/MPGDCylinderBarrelTracker_geo.cpp Outdated Show resolved Hide resolved
@wdconinc
Copy link
Contributor

Updated branch to restart CI pipelines with updated DD4hep installation.

ybedfer and others added 7 commits July 25, 2024 20:22
MPGDCylinderBarrelTracker_geo.cpp": Remove unneeded cast.

Co-authored-by: Wouter Deconinck <[email protected]>
Cosmetics...

Co-authored-by: Wouter Deconinck <[email protected]>
Cast DD4hep units in printout.

Co-authored-by: Wouter Deconinck <[email protected]>
Restricting  scope of ROOT namespace.

Co-authored-by: Wouter Deconinck <[email protected]>
This, to obtain a resolution of about 2cm, i.e. approx., the sum of two neighbouring frames.
@ybedfer
Copy link
Contributor Author

ybedfer commented Jul 27, 2024

Hello,
What could I do concerning the failing check "eicweb/reconstruction_benchmark"? I can see that some (I haven't checked them all) jobs are crashing w/:

#5  0x00007fb9ab939b58 in draw_surfaces(std::shared_ptr<Acts::TrackingGeometry const>, Acts::ContextType, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&) () from /opt/local/lib/libJugTrackPlugins.so

Does this mean that it's connected w/ the specification of the sensitive surface? or the <layer_material/> which I modified lately?
The benchmarks seem to be available on github ("[email protected]:EIC/benchmarks/reconstruction_benchmarks.git"). "https://eicweb.phy.anl.gov/EIC/benchmarks". Can I run them, with a limited amount of training and effort?

@wdconinc
Copy link
Contributor

I noticed the failing reconstruction_benchmarks check yesterday and looked into it. The issue is in legacy code, and stems from an incomplete list of supported surfaces in our export to a 3D graphics file for visualization and debugging. We have fixed the equivalent code in our up-to-date code, and yesterday I propagated a fix to the legacy code as well. It takes a day or so for this to propagate to the containers in which we run the checks, so I suspect this will be resolved by Monday at the latest.

@ybedfer
Copy link
Contributor Author

ybedfer commented Jul 30, 2024

Hello,
There is no progress.
Is it because something went wrong in your fix this business of "incomplete list of supported surfaces"? Or is it simply that the job has not been restarted?

@wdconinc
Copy link
Contributor

I re-triggered the job and it now succeeded. All checks now pass.

@wdconinc wdconinc enabled auto-merge July 30, 2024 18:28
@wdconinc wdconinc added this pull request to the merge queue Jul 30, 2024
Merged via the queue into main with commit 7e32143 Jul 30, 2024
113 checks passed
@wdconinc wdconinc deleted the inner-mpgd-barrel-geoUpdate branch July 30, 2024 21:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
topic: barrel Mid-rapidity detectors topic: forward Positive-rapidity detectors (hadron-going side) topic: tracking
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants