Adding realism to Hcal digitization and Hcal geometry #5

cmantill · 2021-02-08T21:53:32Z

Related to Hcal digitization PR in ldmx-sw

tomeichlersmith

Great work! More detailed discussion on parent PR.

test/hcal_digi_pipeline_test_config.py

test/HcalDigiPipelineTest.cxx

include/Hcal/HcalDigiProducer.h

include/Hcal/HcalOldDigiProducer.h

src/Hcal/HcalGeometryProvider.cxx

src/Hcal/HcalRecProducer.cxx

tomeichlersmith

Review Round 2 🎉

Again, great work. I have two large(ish)-scale comments.

Not sure what is "right", but it seems like the single-ended readout mode should still account for attenuation and time shift as the pulse travels down the bar to the chip.
Don't mix ID types! All of the IDs that you use at your DIGI level (including single-ended readout!) should be digi IDs. Even if you decide to not account for attenuation and not define the end at which the single-ended bars are read-out. The types shouldn't mix.

python/digi.py

src/Hcal/HcalDigiProducer.cxx

src/Hcal/HcalGeometryProvider.cxx

src/Hcal/HcalRecProducer.cxx

test/HcalDigiPipelineTest.cxx

test/hcal_digi_pipeline_test_config.py

tomeichlersmith · 2021-03-16T15:57:59Z

Ok, I'm reporting back here about the testing. I really want to get this working becuase these tests look really useful.

After a clean build of ldmx-sw/iss941 + Hcal/iss1, I also uncommented the following line so that the geometry test would run.

Hcal/test/HcalGeometryTest.cxx

Line 111 in 5a7474b

// p->run();

I have not been able to mimic the issue that you posted?

Maybe there are two errors on top of each other...

[ldmx] tom@sokka:~/ldmx/ldmx-sw/build/test$ ldmx ../run_test [Hcal][Geometry]
Filters: [Hcal][Geometry]
No test cases matched '[Hcal][Geometry]'
===============================================================================
No tests ran

And

[ldmx] tom@sokka:~/ldmx/ldmx-sw/build/test$ ldmx ../run_test [Hcal][functionality]
Filters: [Hcal][functionality]

~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
...
output including failures from digi tests
...
===============================================================================
test cases:    2 |    0 passed |  2 failed
assertions: 7432 | 7419 passed | 13 failed

Ok... so let's try to do the tests individually.

[ldmx] tom@sokka:~/ldmx/ldmx-sw/build/test$ ldmx ../run_test [Hcal] "*Digi*"
Filters: [Hcal] *Digi*

~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
..
12 digi mis-match failures
...
===============================================================================
test cases:    1 |    0 passed |  1 failed
assertions: 6001 | 5989 passed | 12 failed

And

[ldmx] tom@sokka:~/ldmx/ldmx-sw/build/test$ ldmx ../run_test [Hcal] "*Geometry*"
Filters: [Hcal] *Geometry*
... output from running simulation ...
===============================================================================
All tests passed (1430 assertions in 1 test case)

But running them together...

[ldmx] tom@sokka:~/ldmx/ldmx-sw/build/test$ ldmx ../run_test [Hcal]
Filters: [Hcal]

~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
run_test is a Catch v2.13.0 host application.
Run with -? for options

-------------------------------------------------------------------------------
Hcal Digi Pipeline test
-------------------------------------------------------------------------------
/home/tom/ldmx/ldmx-sw/Hcal/test/HcalDigiPipelineTest.cxx:239
...............................................................................

... 12 digi failures ...

... simulation being initialized ...
-------------------------------------------------------------------------------
Hcal Geometry test
-------------------------------------------------------------------------------
/home/tom/ldmx/ldmx-sw/Hcal/test/HcalGeometryTest.cxx:103
...............................................................................

/home/tom/ldmx/ldmx-sw/Hcal/test/HcalGeometryTest.cxx:110: FAILED:
  {Unknown expression after the reported line}
due to a fatal error condition:
  SIGSEGV - Segmentation violation signal

===============================================================================
test cases:    2 |    0 passed |  2 failed
assertions: 7432 | 7419 passed | 13 failed


 *** Break *** segmentation violation
 Generating stack trace...
/home/ldmx.sh: line 60:    39 Segmentation fault      (core dumped) ../run_test [Hcal]

So I have confirmed what you've already told me 🎉

More Investigation

I've found using the -s flag helps. It tells run_test to print successful tests as well as failures. This can tell us when the failure actually happens.

[ldmx] tom@sokka:~/ldmx/ldmx-sw/build/test$ ldmx ../run_test -s [Hcal]
Filters: [Hcal]

~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
run_test is a Catch v2.13.0 host application.
Run with -? for options

-------------------------------------------------------------------------------
Hcal Digi Pipeline test
-------------------------------------------------------------------------------
/home/tom/ldmx/ldmx-sw/Hcal/test/HcalDigiPipelineTest.cxx:239
...............................................................................

... 12 digi failures ...

... simulation being initialized ...
-------------------------------------------------------------------------------
Hcal Geometry test
-------------------------------------------------------------------------------
/home/tom/ldmx/ldmx-sw/Hcal/test/HcalGeometryTest.cxx:103
...............................................................................

... A BUNCH OF SUCCESSFUL POSITION TESTS ...

/home/tom/ldmx/ldmx-sw/Hcal/test/HcalGeometryTest.cxx:110: FAILED:
  {Unknown expression after the reported line}
due to a fatal error condition:
  SIGSEGV - Segmentation violation signal

===============================================================================
test cases:    2 |    0 passed |  2 failed
assertions: 7432 | 7419 passed | 13 failed


 *** Break *** segmentation violation
 Generating stack trace...
/home/ldmx.sh: line 60:    39 Segmentation fault      (core dumped) ../run_test [Hcal]

Ok ... so we are successfully testing a lot of the positions. I put in a print statement at the end of HcalCheckReconstruction::analyze to see if we finish testing the positions and we do. I then added print statements in onProcessEnd and onFileClose to see if we finish processing and we don't. I tried increasing the number of events and we don't get passed the first event, so something is breaking in nextEvent. I will keep looking at this today, super interesting.

tomeichlersmith

I can't find a way to allow both of your tests to run with the ntuplizing uncommented without modifying Framework. I think for now, we should leave the ntuplizing commented-out and then we can uncomment it in the future when a patch to Framework is merged in.

The NtupleManager is a global singleton which is only destroyed when the DLL is unloaded. This means both of the HCal tests see the same NtupleManager object.

LDMX-Software/Framework#39

include/Hcal/HcalDigiProducer.h

omar-moreno · 2021-03-16T19:02:18Z

include/Hcal/HcalRecProducer.h

+  /**
+   * Grabs configure parameters from the python config file.
+   */
+  virtual void configure(framework::config::Parameters&);


See above comment on virtual methods.

src/Hcal/HcalGeometryProvider.cxx

cmantill added 4 commits February 1, 2021 11:07

add test

401db3f

more realistic digis and rechits

84c9a46

clean up gain/thresholds

ddaade2

formatting

121525a

cmantill mentioned this pull request Feb 8, 2021

Adding realism to Hcal digitization and Hcal geometry LDMX-Software/ldmx-sw#958

Merged

tomeichlersmith suggested changes Feb 9, 2021

View reviewed changes

cmantill added 11 commits February 15, 2021 14:52

remove explicit library calling

d82d804

clean up

fb5fc2c

change position to TVector3 and use isNegativeEnd()

b1f1977

remove old producer

392406e

clean up test

265cb43

move attenuation and shift to the digi producer

ff731b1

add noise and include attenuation/shift in simhits

4c9b4b8

fixes to geometry

e582c17

adding comments

bcc7353

formatting

425fa1f

move hgcroc emulator definitions

deb2a21

tomeichlersmith self-requested a review February 25, 2021 21:42

tomeichlersmith suggested changes Feb 25, 2021

View reviewed changes

cmantill added 11 commits February 26, 2021 13:08

single emulator class

fa790d0

add attenuation for sideHcal

0911681

fix number of test hits

72b3c1b

fix ends for sideHcal

31cbb43

add geometry test

9931307

delete unused positionmap from digi

b57e14a

modify name to fakesimhits

d1ad5c1

make the energy test more granular

3d4291e

set gain to 5

afd707e

formatting

909d808

add comment on double number of channels

1857ada

cmantill added 3 commits March 9, 2021 10:19

configure noise generator

c365900

change number of test simhits to 1000

5a7474b

fix noise injector and add noise digis

90d7e63

cmantill added 4 commits March 16, 2021 11:42

lower noise, fix noise hits and reverse voltage att for side HCal

6904b83

comment ntuplizing of energies

0371a03

change analyzer name

65857be

change analyzer in config

f19c670

tomeichlersmith self-requested a review March 16, 2021 18:10

tomeichlersmith mentioned this pull request Mar 16, 2021

Add Total Reset for NtupleManager LDMX-Software/Framework#39

Closed

tomeichlersmith approved these changes Mar 16, 2021

View reviewed changes

omar-moreno approved these changes Mar 16, 2021

View reviewed changes

omar-moreno self-requested a review March 18, 2021 21:17

omar-moreno added 2 commits March 18, 2021 14:20

Remove virtual keyword from methods. Add final override instead.

3fb549a

Remove virtual keyword.

d87b91d

omar-moreno merged commit fc0359c into trunk Mar 18, 2021

omar-moreno mentioned this pull request Mar 18, 2021

Digi improvements #1

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Adding realism to Hcal digitization and Hcal geometry #5

Adding realism to Hcal digitization and Hcal geometry #5

cmantill commented Feb 8, 2021

tomeichlersmith left a comment

tomeichlersmith left a comment

tomeichlersmith commented Mar 16, 2021 •

edited

Loading

tomeichlersmith left a comment

omar-moreno Mar 16, 2021

Adding realism to Hcal digitization and Hcal geometry #5

Adding realism to Hcal digitization and Hcal geometry #5

Conversation

cmantill commented Feb 8, 2021

tomeichlersmith left a comment

Choose a reason for hiding this comment

tomeichlersmith left a comment

Choose a reason for hiding this comment

tomeichlersmith commented Mar 16, 2021 • edited Loading

I have not been able to mimic the issue that you posted?

Ok... so let's try to do the tests individually.

More Investigation

tomeichlersmith left a comment

Choose a reason for hiding this comment

omar-moreno Mar 16, 2021

Choose a reason for hiding this comment

tomeichlersmith commented Mar 16, 2021 •

edited

Loading