DUNE / dunedataprep

Apache License 2.0
0 stars 7 forks source link

FD reco regression CI test fails trying to associate a wire to a raw digit #9

Closed tomjunk closed 2 years ago

tomjunk commented 2 years ago

Recently started failing (Sep. 1, 2022)

23: lar --rethrow-all -n 1 --timing-db time.db -o prodgenie_nue_dune10kt_1x2x6_reco_Current.root --config ci_test_reco_dunefd.fcl xroot://fndca1.fnal.gov:1095/pnfs/fnal.gov/usr/dune/persistent/stash/ContinuousIntegration/DUNEFD/detsim/prodgenie_nue_dune10kt_1x2x6_detsim_Reference.root

61: %MSG-s ArtException: PostEndJob 01-Sep-2022 18:07:17 CDT ModuleEndJob 62: ---- EventProcessorFailure BEGIN 63: EventProcessor: an exception occurred during current event processing 64: ---- ScheduleExecutionFailure BEGIN 65: Path: ProcessingStopped. 66: ---- ProductRegistrationFailure BEGIN 67: Can't associate wire 0 with raw digit 3255The above exception was thrown while processing module DataPrepModule/caldata run: 20000002 subRun: 0 event: 1 68: ---- ProductRegistrationFailure END 69: Exception going through path reco 70: ---- ScheduleExecutionFailure END 71: ---- EventProcessorFailure END 72: %MSG

dladams commented 2 years ago

I see Tom made a mod:

diff --git a/dunedataprep/DataPrep/Module/DataPrepModule_module.cc b/dunedataprep/DataPrep/Module/DataPrepModule_module.cc
index 2f8897f..0d68b2d 100644
--- a/dunedataprep/DataPrep/Module/DataPrepModule_module.cc
+++ b/dunedataprep/DataPrep/Module/DataPrepModule_module.cc
@@ -142,7 +142,11 @@ DataPrepModule::DataPrepModule(fhicl::ParameterSet const& pset) : EDProducer{pse
     }
     cout << myname << "Wires will be saved with name " << m_WireName << endl;
   } else {
-    cout << myname << "Wires will be not be saved." << endl;
+    cout << myname << "Wires will not be saved." << endl;
+    if ( m_DoAssns ) {
+      cout << myname << "But associations of digits and wires were requested.  Overriding: not making associations either." << endl;
+      m_DoAssns = false;
+    }
   }
   for ( string sname : m_IntermediateStates ) {
     if ( m_LogLevel > 0 ) cout << myname << "Module will produce intermediate Wires with name " << sname << endl;
fnal:dune> vim DataPrepModule_module.cc 

and presumably this resolves the issue. Right?

tomjunk commented 2 years ago

This resolves this issue, but the CI test for FD reco now fails with the Gaus Hit finder not finding any wires. I'm guessing that empty string in the wire output just needs to be configured.

tomjunk commented 2 years ago

https://dbweb8.fnal.gov:8443/LarCI/app/ns:dune/storage/docs/2022/09/02/stderr%23BiNaAU1.log

dladams commented 2 years ago

Do we allow saving a container without a name? I had assume not, but if so, I will restore the old behavior.

tomjunk commented 2 years ago

It looks like the WireName string gets used in Event::put() as the instance name. art labels output data products with the module label (in this case caldata), and with the old behavior, the instance name was the empty string, and this worked. I just now pushed changes to dataprep_dune.fcl that set the WireName string to be "dataprep", to match the OutputWireName in another config block in the same file. As long as downstream modules are not relying on an empty instance name, it should be okay. DataPrepModule now cannot emit wires with an emtpy instance name; I'm not sure if that's a problem. I put comments in dataprep_dune.fcl to say "non-blank to write wires"

dladams commented 2 years ago

I have updated the code to add control flags for saving wires and creating associations:

  m_saveWires = m_WireName != "" && m_WireName != "NOSAVE";
  m_associateWires = m_saveWires && m_DoAssns;

so WireName = "" or "NOSAVE" will prevent the saving of wires . We can simply drop the first comparison on the first line to allow blank container names.

dladams commented 2 years ago

I pushed the above change. Tom, please let us know if CI test passes or fails. In the former case, we can close this.

tomjunk commented 2 years ago

Thanks for the update. GausHit is still complaining, so I backed out my changes to dataprep_dune.fcl that set the wire instance name. I'll see if that helps.

tomjunk commented 2 years ago

Should this line

https://github.com/DUNE/dunedataprep/blob/67b64efb086aa28f6b3cbc64b627e84ca5bdeda1/dunedataprep/DataPrep/Module/DataPrepModule_module.cc#L742

be checking the m_AssociateWires flag instead of m_DoAssns ?

dladams commented 2 years ago

Yes, I see I didn't make all the changes. Trying again.

I will also drop the check on blank name so the behavior is as before (last release) except the special name "NOSAVE" stops the saving of wires and building of associations.

dladams commented 2 years ago

Change committed. Again let us know how CI test goes.

tomjunk commented 2 years ago

The FD reco regression CI test succeeded. Thanks! There are some differences in Pandora data product sizes -- numbers of space points and clusters, but these look like old differences not induced by this latest change. Need new reference histograms I think.

tomjunk commented 2 years ago

So I think we're good now, but release v09_58_01d00 swept in the bug. I'll let Jake know.