Project

General

Profile

Support #23196

Make GausHitFinder's dependence on RawDigit optional

Added by Tingjun Yang 4 months ago. Updated about 1 month ago.

Status:
Resolved
Priority:
Normal
Assignee:
Category:
-
Target version:
-
Start date:
08/30/2019
Due date:
% Done:

100%

Estimated time:
Experiment:
DUNE
Co-Assignees:
Duration:

Description

Dear LArSoft experts,

Currently GausHitFinder demands the presence of raw::RawDigits as an input and makes association between recob::Hit and raw::RawDigit. Since most of the experiments drop raw digits during data production and the raw channel information can be obtained through recob::Wire which is also associated with recob::Hit, there is no need to associate raw::RawDigit with recob::Hit.

ProtoDUNE is in the processing of configuring the early reconstruction not to generate raw::RawDigits at all and only save recob::Wire. Tom and David are working on this. This will save memory usage as we don't need to keep the raw::RawDigits in the memory for the entire process, which would be dropped in the end any way. However, we cannot run GuasHitFinder if we do not produce raw::RawDigits.

The forced link between recob::Hit and raw::Digit also leads to other issues. For example, people cannot rerun GausHitFinder on files produced by the production team because the RawDigits are dropped.

Would it be OK to make the dependence of GausHitFinder on RawDigits optional?

Thanks,
Tingjun

History

#1 Updated by Tracy Usher 4 months ago

You might try the larreco feature branch feature/usher_hitfindernorawdigits and see if this does what you want (need to set a fhicl parameter).

#2 Updated by Tingjun Yang 3 months ago

Thanks Tracy. Could this be merged into develop if it is not a breaking change?

#3 Updated by Tracy Usher 3 months ago

Tingjun Yang wrote:

Thanks Tracy. Could this be merged into develop if it is not a breaking change?

I don't believe it is a breaking change. There is an additional fhicl parameter added but the default value reproduces the current behavior of the module so I would not expect any user to see any difference or have to make any changes.

#4 Updated by Kyle Knoepfel 3 months ago

  • Status changed from New to Feedback

We'll run the CI tests for this feature branch. If there are no issues, then we can merge it with this week's LArSoft release.

Tingjun, have you tested this branch, and does it do what you want?

#5 Updated by Tingjun Yang 3 months ago

I believe Bruce tested it and verified it works. Thanks.

#6 Updated by Lynn Garren 3 months ago

  • Assignee set to Tracy Usher
  • Status changed from Feedback to Assigned

Tracy, this branch is out of date with the head of develop. It does not merge with develop and I am having trouble safely identifying the required changes. Are you able to bring it up to date ASAP?

#7 Updated by Lynn Garren 3 months ago

Sorry for the confusion. I checked out feature/usher_hitfinderupdates instead of feature/usher_hitfindernorawdigits. There is no problem with the correct feature branch.

Tracy, would you remove your old branches from the repository? That will help us avoid these problems.

#8 Updated by Kyle Knoepfel 3 months ago

  • % Done changed from 0 to 100
  • Status changed from Assigned to Resolved

#9 Updated by Kyle Knoepfel 3 months ago

  • Status changed from Resolved to Closed

#10 Updated by Tingjun Yang about 2 months ago

  • Status changed from Closed to Feedback

Sorry I finally got a chance to test the feature of running GausHitFinder without raw digits but I got the following error:

%MSG-s ArtException:  PostEndJob 22-Oct-2019 10:16:35 CDT ModuleEndJob
cet::exception caught in art
---- OtherArt BEGIN
  ---- EventProcessorFailure BEGIN
    EventProcessor: an exception occurred during current event processing
    ---- ScheduleExecutionFailure BEGIN
      Path: ProcessingStopped.
      ---- LogicError BEGIN
        Invalid FindOneP
        ---- ProductNotFound BEGIN
          getByLabel: Found zero products matching all criteria
          Looking for type: art::Assns<recob::Hit,raw::RawDigit,void>
          Looking for module label: gaushit
          Looking for productInstanceName: 
        ---- ProductNotFound END
        Attempt to use a FindOneP where the underlying art::Assns product was not found.cet::exception going through module 
      ---- LogicError END
      Exception going through path decode
    ---- ScheduleExecutionFailure END
  ---- EventProcessorFailure END
---- OtherArt END
%MSG
Art has completed and will exit with status 1.

The following line seems to be the problem:

  234     // #################################################################
  235     // ### Reading in the RawDigit associated with these wires, too  ###
  236     // #################################################################
  237     art::FindOneP<raw::RawDigit> RawDigits
  238         (wireVecHandle, evt, fCalDataModuleLabel);

#11 Updated by Tingjun Yang about 2 months ago

Can we remove rawdigit association to gaushit entirely? I believe it is set that way for historical reason and it is not used by any one.

If one needs such an association, one can always go through hit->wire->rawdigit.

#12 Updated by Tracy Usher about 2 months ago

Which module is causing that problem? I think it got cut out when copied here?

#13 Updated by Tingjun Yang about 2 months ago

It is GausHitFinder_module.cc, line 237.

#14 Updated by Tracy Usher about 2 months ago

I would certainly agree that the usage is historical and I'm not sure I know of a use case currently active for these associations... I guess this slipped through the testing somehow (I thought it had been shown to work?), but I can see an easy one line "kludge" to get past the current issue until there is agreement to drop hit<-->RawDigit associations completely...

#15 Updated by Tingjun Yang about 2 months ago

Hi Tracy,

Do you have a solution to this issue? Protodune is waiting for this to be resolved to proceed with tool based raw decoding.

Thanks,
Tingjun

#16 Updated by Tingjun Yang about 2 months ago

If it is not easy to make the dependence rawdigits optional, can we agree to remove the association to rawdigits completely? Thanks.

#17 Updated by Tracy Usher about 2 months ago

Tingjun Yang wrote:

If it is not easy to make the dependence rawdigits optional, can we agree to remove the association to rawdigits completely? Thanks.

Sorry, did not realize ProtoDUNE was being held up and it was urgent, only now seeing this... sorry! I am unfortunately not in position to do something for the next 4-5 hours, can look at after. Probably best solution is, as you say, to remove the associations outright.

#18 Updated by Tracy Usher about 1 month ago

I apologize for the delay!
I have prepared a feature branch, feature/usher_removerawdigits, which removes the code for making RawDigit<-->Hit associations.

Tracy

#19 Updated by Lynn Garren about 1 month ago

  • Status changed from Feedback to Assigned

We will schedule larreco feature/usher_removerawdigits for inclusion in this weeks' larsoft release. I fired off a CI.

#20 Updated by Lynn Garren about 1 month ago

I made local test build which includes the experiment code (excepting icaruscode). This build uses the sbndcode v08_34_00 tag. After disabling the uboonecode overlay test (redundant and too long), only one test fails: uboonecode production_mcc9.sh.

lar --rethrow-all -c reco_uboone_mcc9_8_driver_stage2.fcl -s reco_uboone_mcc9_8_driver_stage1.root -o reco_uboone_mcc9_8_driver_stage2.root -n 5

.....

%MSG-s ArtException:  PostEndJob 04-Nov-2019 16:24:45 CST ModuleEndJob
cet::exception caught in art
---- OtherArt BEGIN
  ---- EventProcessorFailure BEGIN
    EventProcessor: an exception occurred during current event processing
    ---- ScheduleExecutionFailure BEGIN
      Path: ProcessingStopped.
      ---- LogicError BEGIN
        Invalid FindOneP
        ---- ProductNotFound BEGIN
          getByLabel: Found zero products matching all criteria
          Looking for type: art::Assns<recob::Hit,raw::RawDigit,void>
          Looking for module label: gaushit
          Looking for productInstanceName: 
        ---- ProductNotFound END
        Attempt to use a FindOneP where the underlying art::Assns product was not found.cet::exception going through module 
      ---- LogicError END
      Exception going through path reco
    ---- ScheduleExecutionFailure END
  ---- EventProcessorFailure END
  ---- OtherArt BEGIN
    ---- FatalRootError BEGIN
      Fatal Root Error: @SUB=TTree::SetEntries
      Tree branches have different numbers of entries, eg anab::CosmicTagrecob::Trackvoidart::Assns_pandoraCosmicFlashTag__McRecoStage2. has 0 entries while EventAuxiliary has 5 entries.
    ---- FatalRootError END
  ---- OtherArt END
---- OtherArt END
%MSG

#21 Updated by Lynn Garren about 1 month ago

This will be included in the upcoming larsoft v08_35_00 release. There are two feature branches:
  • larreco feature/usher_removerawdigits
  • larana feature/usher_removeRawDigits

#22 Updated by Lynn Garren about 1 month ago

  • Status changed from Assigned to Resolved

larsoft v08_35_00 is now available with this fix.



Also available in: Atom PDF