Origin/calo geom g4 update #1311

bechenard · 2024-07-16T20:06:24Z

Updated description of the calorimeter with latest geometry drawings. I changed the way the cable runs were coded to insulate the calorimeter from the rest of the DS vaccum. I also added support for realistic crystal placement and checks to avoid crystal overlaps.

FNALbuild · 2024-07-16T20:06:50Z

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

GeometryService
Mu2eG4Helper
DetectorSolenoidGeom
Mu2eG4
CalorimeterGeom
Analyses

which require these tests: build.

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

The following users requested to be notified about changes to these packages:
@resnegfk

⌛ The following tests have been triggered for d41d099: build (Build queue has 1 jobs)

About FNALbuild. Code review on Mu2e/Offline.

FNALbuild · 2024-07-16T20:16:31Z

☔ The build tests failed for d41d099.

Test	Result	Details
test with	✅	Command did not list any other PRs to include
merge	✅	Merged `d41d099` at `af80f04`
build (prof)	✅	Log file. Build time: 04 min 12 sec
ceSimReco	❌	Log file. Return Code 1.
g4test_03MT	✅	Log file.
transportOnly	✅	Log file.
POT	✅	Log file.
g4study	✅	Log file.
cosmicSimReco	❌	Log file. Return Code 1.
cosmicOffSpill	✅	Log file.
ceSteps	❌	Log file. Return Code 1.
ceDigi	❌	Log file. Return Code 24.
muDauSteps	✅	Log file.
ceMix	❌	Log file. Return Code 24.
rootOverlaps	❌	Log file. Return Code 1.
g4surfaceCheck	❌	Log file. Return Code 0.
FIXME, TODO	🔶	TODO (0) FIXME (5) in 31 files
clang-tidy	🔶	0 errors 2434 warnings
whitespace check	✅	no whitespace errors found

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

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.

kutschke · 2024-07-16T20:38:34Z

@bechenard CalorimeterMother was originally defined as the volume that George allocated to the calorimeter. It was a way to enforce that the geometry was compliant with space allocations. Have things now changed so that some volumes (cable runs?) need to cross that boundary? If so, that's Ok but I want to make sure that the we are not getting rid of CalorimeterMother without a good reason.

bechenard · 2024-07-16T21:14:56Z

HI Rob, The full CalorimeterMother volume interfered with the tracker cable run, and this made the implementation unnecessary complicated. Given the "weird" calo geometry, I split the mother volume into a disk mother volume, and a FEB mother volume. Both volume have the same z length as CalorimeterMother and the radial extent of the FEB mother volume matches that of the calorimeterMother. If you wish, I can group the the disk mother and FEB mother into a new calorimeter mother (as a combination of two volumes). Removing CalorimeterMother also crated some issues with the scripts, so adding one back would simplify tests. I;m also addressing the clang tidy warnings. Let me know if there is anything else I should address at the conceptual level. Cheers, B

On 7/16/2024 1:38 PM, Rob Kutschke wrote: @bechenard <https://github.com/bechenard> CalorimeterMother was originally defined as the volume that George allocated to the calorimeter. It was a way to enforce that the geometry was compliant with space allocations. Have things now changed so that some volumes (cable runs?) need to cross that boundary? If so, that's Ok but I want to make sure that the we are not getting rid of CalorimeterMother without a good reason. — Reply to this email directly, view it on GitHub <#1311 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ABJ27JDLF5TOUN76LQX3ZZDZMWAGBAVCNFSM6AAAAABK7IRG4OVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDEMZRG44DONRXGY>. You are receiving this because you were mentioned.Message ID: ***@***.***>

--------------eQSbmgNuM9Mee5dUPtmisFK0 Content-Type: text/html; charset=UTF-8 Content-Transfer-Encoding: 8bit <!DOCTYPE html><html><head> <meta http-equiv="Content-Type" content="text/html; charset=utf-8"> </head> <body> HI Rob, The full CalorimeterMother volume interfered with the tracker cable run, and this made the implementation unnecessary complicated. Given the "weird" calo geometry, I split the mother volume into a disk mother volume, and a FEB mother volume. Both volume have the same z length as CalorimeterMother and the radial extent of the FEB mother volume matches that of the calorimeterMother. If you wish, I can group the the disk mother and FEB mother into a new calorimeter mother (as a combination of two volumes). Removing CalorimeterMother also crated some issues with the scripts, so adding one back would simplify tests. I;m also addressing the clang tidy warnings. Let me know if there is anything else I should address at the conceptual level. Cheers, B <div class="moz-cite-prefix">On 7/16/2024 1:38 PM, Rob Kutschke wrote: </div> <blockquote type="cite" ***@***.***"> <a class="user-mention notranslate" data-hovercard-type="user" data-hovercard-url="/users/bechenard/hovercard" data-octo-click="hovercard-link-click" data-octo-dimensions="link_type:self" href="https://github.com/bechenard" ***@***.***</a> CalorimeterMother was originally defined as the volume that George allocated to the calorimeter. It was a way to enforce that the geometry was compliant with space allocations. Have things now changed so that some volumes (cable runs?) need to cross that boundary? If so, that's Ok but I want to make sure that the we are not getting rid of CalorimeterMother without a good reason. — Reply to this email directly, <a href="#1311 (comment)" moz-do-not-send="true">view it on GitHub</a>, or <a href="https://github.com/notifications/unsubscribe-auth/ABJ27JDLF5TOUN76LQX3ZZDZMWAGBAVCNFSM6AAAAABK7IRG4OVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDEMZRG44DONRXGY" moz-do-not-send="true">unsubscribe</a>. You are receiving this because you were mentioned.<img src="https://github.com/notifications/beacon/ABJ27JCY6RNGKCWRQFUUIT3ZMWAGBA5CNFSM6AAAAABK7IRG4OWGG33NNVSW45C7OR4XAZNMJFZXG5LFINXW23LFNZ2KUY3PNVWWK3TUL5UWJTUFAZQJY.gif" alt="" moz-do-not-send="true" width="1" height="1">Message ID: <Mu2e/Offline/pull/1311/c2231787676@github.com> <script type="application/ld+json">[ { ***@***.***": "http://schema.org", ***@***.***": "EmailMessage", "potentialAction": { ***@***.***": "ViewAction", "target": "#1311 (comment)", "url": "#1311 (comment)", "name": "View Pull Request" }, "description": "View this Pull Request on GitHub", "publisher": { ***@***.***": "Organization", "name": "GitHub", "url": "https://github.com" } } ]</script> </blockquote> </body> </html>

…

--------------eQSbmgNuM9Mee5dUPtmisFK0--

kutschke · 2024-07-16T21:33:31Z

HI Rob, The full CalorimeterMother volume interfered with the tracker cable run, and this made the implementation unnecessary complicated. Given the "weird" calo geometry, I split the mother volume into a disk mother volume, and a FEB mother volume. Both volume have the same z length as CalorimeterMother and the radial extent of the FEB mother volume matches that of the calorimeterMother. If you wish, I can group the the disk mother and FEB mother into a new calorimeter mother (as a combination of two volumes). Removing CalorimeterMother also crated some issues with the scripts, so adding one back would simplify tests. I;m also addressing the clang tidy warnings. Let me know if there is anything else I should address at the conceptual level.

The CalorimeterMother is also used to set UserLimits and some of the stepping cuts. It's referenced in some of the fcl in Production. At one time I think it was used as a stopping volume in the cosmic workflows but I think that's no longer true - check with Yuri and Ralf.

Your reason for removing it (volumes that need to cross it's boundaries ) is OK with me so long as you can manage all of the downstream effects.

About the clang tidy warnings - deal with the ones from your own code. Most will come from included headers - many of which I wrote.

FNALbuild · 2024-07-17T18:21:24Z

📝 The HEAD of main has changed to c98a795. Tests are now out of date.

kutschke · 2024-07-18T19:29:09Z

@FNALbuild run build test

FNALbuild · 2024-07-18T19:29:31Z

⌛ The following tests have been triggered for 8afaff6: build (Build queue has 1 jobs)

FNALbuild · 2024-07-18T19:45:19Z

☔ The build tests failed for 8afaff6.

Test	Result	Details
test with	✅	Command did not list any other PRs to include
merge	✅	Merged `8afaff6` at `a597848`
build (prof)	✅	Log file. Build time: 08 min 26 sec
ceSimReco	❌	Log file. Return Code 134.
g4test_03MT	✅	Log file.
transportOnly	✅	Log file.
POT	❌	Log file. Return Code 134.
g4study	✅	Log file.
cosmicSimReco	❌	Log file. Return Code 134.
cosmicOffSpill	✅	Log file.
ceSteps	❌	Log file. Return Code 134.
ceDigi	❌	Log file. Return Code 24.
muDauSteps	✅	Log file.
ceMix	❌	Log file. Return Code 24.
rootOverlaps	❌	Log file. Return Code 134.
g4surfaceCheck	❌	Log file. Return Code 134.
FIXME, TODO	🔶	TODO (0) FIXME (4) in 31 files
clang-tidy	🔶	0 errors 2268 warnings
whitespace check	✅	no whitespace errors found

N.B. These results were obtained from a build of this Pull Request at 8afaff6 after being merged into the base branch at a597848.

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.

bechenard · 2024-07-19T00:17:09Z

@FNALbuild run build test

FNALbuild · 2024-07-19T00:17:33Z

⌛ The following tests have been triggered for 2f968ed: build (Build queue has 1 jobs)

kutschke · 2024-07-24T03:00:43Z

I rebuilt and reran ceSimReco. This time it threw an exception after 490 events:

%MSG-s ArtException:  PostEndJob 23-Jul-2024 21:54:16 CDT ModuleEndJob
---- EventProcessorFailure BEGIN
  EventProcessor: an exception occurred during current event processing
  ---- ScheduleExecutionFailure BEGIN
    Path: ProcessingStopped.
    ---- StdException BEGIN
      An exception was thrown while processing module CaloShowerStepMaker/CaloShowerStepMaker run: 1200 subRun: 0 event: 490
      vector::_M_range_check: __n (which is 2583) >= this->size() (which is 2437)
    ---- StdException END
    Exception going through path TriggerPath
  ---- ScheduleExecutionFailure END
---- EventProcessorFailure END
%MSG

See: /exp/mu2e/app/users/kutschke/Development/PR1311/pr/ceSimReco.log.2 .

The command that I ran was:
time mu2e -c Production/Validation/ceSimReco.fcl -n 2000 >& ceSimReco.log.2

The Offline was made by:
git checkout -b testmerge3 origin/main
git merge bertrand/origin/CaloGeomG4Update

where origin is Mu2e/Offline at PR 1313.

bechenard · 2024-07-27T01:21:53Z

@FNALbuild run build test

FNALbuild · 2024-07-27T01:22:19Z

⌛ The following tests have been triggered for d182db8: build (Build queue is empty)

FNALbuild · 2024-07-27T01:32:04Z

☀️ The build tests passed at d182db8.

Test	Result	Details
test with	✅	Command did not list any other PRs to include
merge	✅	Merged `d182db8` at `2540af1`
build (prof)	✅	Log file. Build time: 04 min 12 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.
FIXME, TODO	🔶	TODO (4) FIXME (20) in 45 files
clang-tidy	🔶	0 errors 4974 warnings
whitespace check	✅	no whitespace errors found

N.B. These results were obtained from a build of this Pull Request at d182db8 after being merged into the base branch at 2540af1.

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.

bechenard · 2024-07-27T01:56:11Z

@kutschke All good, ready to merge. It turns out it was more confusing to get all signed/unsigned rather than using to const_cast, so I kept all integers and added the const_cast. I need a few more GOTOs, but this is for another PR!

kutschke · 2024-07-27T04:01:06Z

@FNALbuild run build test

FNALbuild · 2024-07-27T04:01:35Z

⌛ The following tests have been triggered for d182db8: build (Build queue is empty)

FNALbuild · 2024-07-27T04:11:27Z

☀️ The build tests passed at d182db8.

Test	Result	Details
test with	✅	Command did not list any other PRs to include
merge	✅	Merged `d182db8` at `e8fecef`
build (prof)	✅	Log file. Build time: 04 min 12 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.
FIXME, TODO	🔶	TODO (4) FIXME (20) in 45 files
clang-tidy	🔶	0 errors 4974 warnings
whitespace check	✅	no whitespace errors found

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

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.

kutschke · 2024-07-29T16:00:23Z

I did valCompare on reco_00.fcl + _01.fcl and on ceSimReco_00.fcl + _01.fcl . The results are at:

https://home.fnal.gov/~kutschke/Mu2e/PR1311/reco/results.html
https://home.fnal.gov/~kutschke/Mu2e/PR1311/ceSimReco/results.html

This is 2x the stats as before. Both look Ok to me. The reco input files are 4 files from dig.mu2e.CeEndpointMix1BBSignal.MDC2020r_best_v1_0 .

There are bigger changes in ceSimReco than in reco, presumably because of random number changes.

Bertrand: please let us know whether or not the comparisons pass your inspection.

brownd1978 · 2024-07-29T19:59:19Z

I'm guessing the change in timing: https://home.fnal.gov/~kutschke/Mu2e/PR1311/reco/Validation_KalSeeds_KKDeM_noName_CCDt.gif comes from changing the calorimeter crystal positions. So this change is NOT compatible with MDC2020 datasets. We will discuss on Wednesday the current plan for branching incompatible changes, I ask that merging be delayed until after that.

…

On Mon, Jul 29, 2024 at 9:01 AM Rob Kutschke ***@***.***> wrote: ***@***.**** approved this pull request. — Reply to this email directly, view it on GitHub <#1311 (review)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ABAH572R3NRMPVINE4537ALZOZRMHAVCNFSM6AAAAABK7IRG4OVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZDEMBVGM4DKNBXHE> . You are receiving this because you are on a team that was mentioned.Message ID: ***@***.***>

-- David Brown ***@***.*** Office Phone (510) 486-7261 Lawrence Berkeley National Lab M/S 50R5008 (50-6026C) Berkeley, CA 94720

kutschke · 2024-07-29T20:16:29Z

I ask that merging be delayed until after that.

That's my plan.

FNALbuild · 2024-09-10T18:28:16Z

📝 The HEAD of main has changed to e389271. Tests are now out of date.

echenard added 6 commits July 12, 2024 18:02

Calorimeter geom update - temp

238e82e

Calorimeter geom update part II

496acbe

Calorimeter geom update part III

7636ce4

Calorimeter geom update part IV

fb690c6

Small fix in calo geom file

ccd79c3

Calo geom additional fixes

d41d099

FNALbuild added the build running label Jul 16, 2024

FNALbuild added build failed and removed build running labels Jul 16, 2024

FNALbuild added build pending and removed build failed labels Jul 17, 2024

Reintegrated calorimeterMother volume and fixed a few small issues

8afaff6

FNALbuild added build running and removed build pending labels Jul 18, 2024

FNALbuild added build failed and removed build running labels Jul 18, 2024

Fixed memory issue

2f968ed

FNALbuild added build pending and removed build failed labels Jul 19, 2024

Added robust checks to calo code

3b589fb

FNALbuild added build pending and removed build finished labels Jul 27, 2024

echenard added 2 commits July 26, 2024 20:05

Fixed a few spaces

b128d3c

Fixed static_cast

d182db8

FNALbuild added build running and removed build pending labels Jul 27, 2024

FNALbuild added build finished and removed build running labels Jul 27, 2024

FNALbuild added build pending build running and removed build finished build pending labels Jul 27, 2024

FNALbuild added build finished and removed build running labels Jul 27, 2024

kutschke approved these changes Jul 29, 2024

View reviewed changes

FNALbuild added build pending and removed build finished labels Sep 10, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Origin/calo geom g4 update #1311

Origin/calo geom g4 update #1311

bechenard commented Jul 16, 2024

FNALbuild commented Jul 16, 2024

FNALbuild commented Jul 16, 2024

kutschke commented Jul 16, 2024

bechenard commented Jul 16, 2024 via email

kutschke commented Jul 16, 2024

FNALbuild commented Jul 17, 2024

kutschke commented Jul 18, 2024

FNALbuild commented Jul 18, 2024

FNALbuild commented Jul 18, 2024

bechenard commented Jul 19, 2024

FNALbuild commented Jul 19, 2024

kutschke commented Jul 24, 2024

bechenard commented Jul 27, 2024

FNALbuild commented Jul 27, 2024

FNALbuild commented Jul 27, 2024

bechenard commented Jul 27, 2024

kutschke commented Jul 27, 2024

FNALbuild commented Jul 27, 2024

FNALbuild commented Jul 27, 2024

kutschke commented Jul 29, 2024

brownd1978 commented Jul 29, 2024 via email

kutschke commented Jul 29, 2024

FNALbuild commented Sep 10, 2024

Origin/calo geom g4 update #1311

Are you sure you want to change the base?

Origin/calo geom g4 update #1311

Conversation

bechenard commented Jul 16, 2024

FNALbuild commented Jul 16, 2024

FNALbuild commented Jul 16, 2024

kutschke commented Jul 16, 2024

bechenard commented Jul 16, 2024 via email

kutschke commented Jul 16, 2024

FNALbuild commented Jul 17, 2024

kutschke commented Jul 18, 2024

FNALbuild commented Jul 18, 2024

FNALbuild commented Jul 18, 2024

bechenard commented Jul 19, 2024

FNALbuild commented Jul 19, 2024

kutschke commented Jul 24, 2024

bechenard commented Jul 27, 2024

FNALbuild commented Jul 27, 2024

FNALbuild commented Jul 27, 2024

bechenard commented Jul 27, 2024

kutschke commented Jul 27, 2024

FNALbuild commented Jul 27, 2024

FNALbuild commented Jul 27, 2024

kutschke commented Jul 29, 2024

brownd1978 commented Jul 29, 2024 via email

kutschke commented Jul 29, 2024

FNALbuild commented Sep 10, 2024