Project

General

Profile

Bug #15012

Job crashes if no photon backtracker information is available

Added by Tingjun Yang over 2 years ago. Updated over 2 years ago.

Status:
Closed
Priority:
Normal
Assignee:
Category:
Simulation
Target version:
-
Start date:
01/05/2017
Due date:
% Done:

0%

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

Description

There seems to be a problem related to photon backtracker. If photon backtracker was not run in largeant stage, any later larsoft jobs will fail when trying to initialize the photon backtracker service. Here is one example:

lar -c /dune/app/users/wallbank/larsoft-base/workspace/job/mva_nue_train_dune10kt_1x2x6.fcl -s /pnfs/dune/scratch/users/wallbank/v06_17_00/reco/prodgenie_nu_dune10kt_1x2x6_reco/12725743_0/v06_11_00_A_prodgenie_nu_dune10kt_1x2x6_519_20161018T115713_detsim_reco.root
...
Begin processing the 1st record. run: 20000001 subRun: 308 event: 30701 at 05-Jan-2017 10:07:28 CST
%MSG-s ArtException:  PostPathEndRun end_path 05-Jan-2017 10:07:29 CST  PostEndRun
cet::exception caught in art
---- EventProcessorFailure BEGIN
  An exception occurred during current event processing
  ---- ProductNotFound BEGIN
    getView: Found no products matching all criteria
    Looking for sequence of type: sim::OpDetBacktrackerRecord
    Looking for module label: largeant
    Looking for productInstanceName: 
  ---- ProductNotFound END
  cet::exception caught in EventProcessor and rethrown
---- EventProcessorFailure END
%MSG
Art has completed and will exit with status 65.
...

Since there are many old MC files without photon backtracker information, this can cause trouble for people to analyze old MC files. I am going to disable photon backtracker in the standard dune services until this is resolved.

Tingjun

History

#1 Updated by Gianluca Petrillo over 2 years ago

  • Description updated (diff)

(added preformatted block to the description)

#2 Updated by Lynn Garren over 2 years ago

There is a page documenting changes to the code in v06_18_01 over v06_18_00. It may be of use, since it includes photon backtracker changes: Explicit_code_changes_since_v06_18_00

#3 Updated by Gianluca Petrillo over 2 years ago

  • Category set to Simulation
  • Status changed from New to Accepted
  • Occurs In v06_17_00 added

First of all: the photon backtracker source reports the name of Brian Rebel. That needs to be fixed since Brian is not the author or the maintainer of this code. Please correct me here if I am wrong.

Second question: the photon backtracker code was duplicated from BackTracker service.
  • is it duplicating functionality of BackTracker?
  • is there any unique functionality of the photon backtracker that does not require sim::OpDetBacktrackerRecord?

If the answer to the second question is no, then I think the solution is that the code which needs functionality also present in BackTracker should use that one instead, and that if photon backtracker service is configured it should require the presence of sim::OpDetBacktrackerRecord.

If the photon backtracker does provide unique functionality that does not require sim::OpDetBacktrackerRecord, then we can change the code to throw an exception only at the time that functionality is used.

Jason, please chime in with information.

#4 Updated by Jason Stock over 2 years ago

I updated the author information for the Photon backtracker. It was an artifact of starting from the Backtracker.

The Photon Backtracker uses most of the same methods as the backtracker, but modified for using OpDetBacktrackerRecords instead of SimChannels.

So, the short answer is everything in PhotonBackTracker requires sim::OpDetBackTrackerRecords. It should absolutely (if configured) require sim::OpDetBackTrackerRecords.

#5 Updated by Jason Stock over 2 years ago

I have implemented a fix in the backtracker that should handle the absence of OpDetBackTrackerRecords more gracefully.
It will now produce a Warning for the user that the PhotonBackTracker will fail on that event and move on.

This warning will be produced once per LArSoft invocation. A message will appear in the debug log for each event that can't be rebuilt.

As soon as this fix hits the next weekly LArSoft build I will turn the photonbacktracker back on in dunetpc.

larsim:59fa2e5, larsim:7234038b develop->develop

#6 Updated by Tingjun Yang over 2 years ago

Thanks Jason for fixing this!

#7 Updated by Gianluca Petrillo over 2 years ago

I have some comments on the commit1 larsim:7234038b:
  1. do not catch all exception, because sooner or later you'll end up catching something unexpected and handling it in the wrong way: in
        try{evt.getView(fG4ModuleLabel, cOpDetBacktrackerRecords);}
        catch(...) {
          // ...
        }
    you are actually looking for:
        try{evt.getView(fG4ModuleLabel, cOpDetBacktrackerRecords);}
        catch(art::Exception const& e) {
          if (e.categoryCode() != art::errors::ProductNotFound) throw; // handle only ProductNotFound
          // ...
        }
  2. static local variables are trouble for so many aspects. Here you can use a data member.
  3. more important: when there is no data product, PhotonBackTracker::TrackIDToSimSDP() silently gives you the wrong result (cOpDetBacktrackerRecords collection stays empty, so this method returns an empty vector).

In general, I invite you to reconsider solving the problem at configuration level (don't need it $\
ightarrow$ don't configure it; need it ["$\\rightarrow$"] immediately require its prerequisites to be satisfied).


1 To refer to a commit, you can use the syntax repository:commit:shortenedcommithash (for example, larsim:commit:7234038b above). It will take a while for Redmine to recognise new commits, but it will eventually happen.

#8 Updated by Jason Stock over 2 years ago

I have addressed your immediate concerns (Bad practice catching all errors, bad practice using static local variable).

#9 Updated by Alexander Himmel over 2 years ago

Gianluca, I'm not sure I follow what you mean by "solve at configuration time." The nature of the backtracker code (as with so many things) is that it will likely need to be run during a "slow" step (say, reconstruction) but a user might not know it is needed until they are trying to understand something that looks strange in a final analysis ntuple, at which point it is a major hassle to re-run everything with the backtracker included. So, my preference is that the PhotonBacktracker should be on and available by default, and only not produce the required data products if it is unable to because it is missing the pre-requisites. In practice, I think this is primarily an issue of backwards compatibility during a transition period since we envision future MC should be generated with PhotonBacktracker information available by default. Do you think this solution is insufficient for that described usage model?

#10 Updated by Jason Stock over 2 years ago

  • Assignee set to Jason Stock

#11 Updated by Jason Stock over 2 years ago

After talking with Gianlucca we have decided on a best practices solution to this issue that meets the usage model will be to make sure the BackTracker differentiates between not finding an entry in SimOpDetBacktrackerRecrods for a given call and there not being a SimOpDetBacktrackerRecords to search through for a given call. The later case will throw an exception, while the former will continue to only throw a warning or message.

This superior solution has not yet been implemented.

#12 Updated by Jason Stock over 2 years ago

I have changed the method of retrieving data from getView to using art Handles instead. This will allow error handling without the use of try{}catch(){} and will simplify the logic of throwing an exception when a user relies on backtracker methods when no backtracker products exist. A flag will be added to throw an exception if ever a method is used while there are no "OpDetBackTrackerRecords".

#13 Updated by Jason Stock over 2 years ago

Commited fixes and updates to PhotonBackTracker in larsim commit a23b251..8c47254 develop->develop. This issue is ready to be marked as "resolved" with the next dunetpc release. I don't seem to have the option to mark it as "resolved" under status.

#14 Updated by Jason Stock over 2 years ago

  • Status changed from Accepted to Assigned

#15 Updated by Tingjun Yang over 2 years ago

  • Status changed from Assigned to Resolved

#16 Updated by Jason Stock over 2 years ago

  • Status changed from Resolved to Closed


Also available in: Atom PDF