JeffersonLab / halld_recon

Reconstruction for the GlueX Detector
6 stars 8 forks source link

User plugin is not working with the newest halld_recon/root version set #584

Closed igjaegle closed 2 years ago

igjaegle commented 2 years ago

Only the histograms in the first loop are filled, all the other are not even filled.

igjaegle commented 2 years ago

I put an plugin example here: /work/halld/home/ijaegle/public/ForMarkI/my-dc The plugin is working with older version of halld_recon but not with the newest version of halld_recon and the new data set (81xxx).

igjaegle commented 2 years ago

The bug is reproducible with version_5.0.1.xml or centrally install hd_recon

sdobbs commented 2 years ago

which loop is the first loop?

igjaegle commented 2 years ago

It is the trigger loop filling the trigger bit of the event

sdobbs commented 2 years ago

thanks - did you check to see if events are being rejected at line 105?

igjaegle commented 2 years ago

Yes, I did write cout at different levels which output the expected numbers

igjaegle commented 2 years ago

NB with an older version of version.xml, it is working properly

aaust commented 2 years ago

I just ran the plugin with the current default version.xml over 2017 data and all histograms were filled. You need to be even more specific what the problem is.

igjaegle commented 2 years ago

It is not working with the new PrimEx-eta data set

sdobbs commented 2 years ago

I could reproduce this with /cache/halld/RunPeriod-2021-08/rawdata/Run081591/hd_rawdata_081591_000.evio, also saw the following crash with HLDetectorTiming:

===========================================================
#6  0x00007efdbaf860fe in SortDirectories () at /u/group/halld/Software/builds/Linux_CentOS7.7-x86_64-gcc4.8.5/halld_recon/halld_recon-4.29.1/Linux_CentOS7.7-x86_64-gcc4.8.5/include/HistogramTools.h:675
#7  0x00007efdbaf86169 in JEventProcessor_HLDetectorTiming::fini (this=) at plugins/Calibration/HLDetectorTiming/JEventProcessor_HLDetectorTiming.cc:1323
#8  0x00000000011882d2 in jana::JApplication::Fini (this=0x7ffeab2879c0, check_fini_called_flag=) at src/JANA/JApplication.cc:1921
#9  0x0000000001185db0 in jana::JApplication::Run (this=this
entry=0x7ffeab2879c0, proc=proc
entry=0x46763a0, Nthreads=, Nthreads
entry=0) at src/JANA/JApplication.cc:1846
#10 0x000000000067abec in main (narg=5, argv=0x7ffeab288028) at programs/Analysis/hd_root/hd_root.cc:46
===========================================================
sdobbs commented 2 years ago

OK, I think I see what is happening... Sometime after the call to Fill1DHistogram(), gDirectory gets changed from the open ROOT file on disk to the pure memory storage. It's not obvious to me when this happens, though. Weird.

Anyway, understanding and fixing this is going to be nontrivial.

The easy fix is to just not use the HistogramTools.h functions for now in your analysis plugin.

sdobbs commented 2 years ago

So, to help with ongoing analysis/calibration work, I just pushed a change to HistogramTools.h on the branch sdobbs_ugly_histo_kludge which lets these functions work. I REALLY don't suggest merging this change, though, since it relies on some assumption which may or may not be true moving forward - we still don't really understand why gDirectory changes.

I can see a few ways forward:

  1. Deprecate HistogramTools.h and require people to allocate all histograms in the init() function in their plugins, as is standard
  2. Make some more robust feature for saving a pointer to the main output file
  3. Maybe take another look at this set of utility functions and generally make them more robust
sdobbs commented 2 years ago

See #587 - @igjaegle please test that this works for you.

aaust commented 2 years ago

I tested it with the latest nightly build and see all histograms.

 gxenv /u/scratch/gluex/nightly/2021-10-28/Linux_CentOS7.7-x86_64-gcc4.8.5/version_2021-10-28.xml

 hd_root -PPLUGINS=my-dc /cache/halld/RunPeriod-2021-08/rawdata/Run081595/hd_rawdata_081595_000.evio

 root hd_root.root
 [1] .ls
 TFile**         hd_root.root    Produced by hd_root
 TFile*         hd_root.root    Produced by hd_root
  KEY: TDirectoryFile   trg;1   trg
  KEY: TDirectoryFile   vertex;1        vertex
  KEY: TDirectoryFile   eta2g;1 eta2g
  KEY: TDirectoryFile   tof;1   tof
igjaegle commented 2 years ago

I confirm it is working also for me.

tks ig.

On Thu, Oct 28, 2021 at 9:09 AM Alex Austregesilo @.***> wrote:

I tested it with the latest nightly build and see all histograms.

gxenv /u/scratch/gluex/nightly/2021-10-28/Linux_CentOS7.7-x86_64-gcc4.8.5/version_2021-10-28.xml

hd_root -PPLUGINS=my-dc /cache/halld/RunPeriod-2021-08/rawdata/Run081595/hd_rawdata_081595_000.evio

root hd_root.root [1] .ls TFile* hd_root.root Produced by hd_root TFile hd_root.root Produced by hd_root KEY: TDirectoryFile trg;1 trg KEY: TDirectoryFile vertex;1 vertex KEY: TDirectoryFile eta2g;1 eta2g KEY: TDirectoryFile tof;1 tof

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/JeffersonLab/halld_recon/issues/584#issuecomment-953829044, or unsubscribe https://github.com/notifications/unsubscribe-auth/AFL3GRV3UW6WTXYO3KXSC2DUJFDSPANCNFSM5GYHBHMQ . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

sdobbs commented 2 years ago

Thanks, let's close this issue then.