Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Reading old files containing std::auto_ptr<gen::PdfInfo> #43422

Open
makortel opened this issue Nov 28, 2023 · 20 comments
Open

Reading old files containing std::auto_ptr<gen::PdfInfo> #43422

makortel opened this issue Nov 28, 2023 · 20 comments

Comments

@makortel
Copy link
Contributor

PR #43405 reminded us about the iorules in SimDataFormats/GeneratorProducts/src/classes_def.xml to read in std::auto_ptr<gen::PdfInfo> from files predating CMSSW_11_0_0. The std::auto_ptr was removed in C++17, although the libstdc++ still provides the implementation for backwards compatibility (link) while issuing a deprecation warning. We should nevertheless figure out a way to not have to use std::auto_ptr in the dictionaires (e.g. if we would have to use a standard library without std::auto_ptr some day).

For reference, here are pointers to various discussions from the time the iorules to convert the std::auto_ptr<gen::PdfInfo> to std::unique_ptr<gen::PdfInfo> were introduced

@makortel
Copy link
Contributor Author

assign core

@cmsbuild
Copy link
Contributor

New categories assigned: core

@Dr15Jones,@makortel,@smuzaffar you have been requested to review this Pull request/Issue and eventually sign? Thanks

@cmsbuild
Copy link
Contributor

cmsbuild commented Nov 28, 2023

A new Issue was created by @makortel Matti Kortelainen.

@makortel, @Dr15Jones, @rappoccio, @antoniovilela, @sextonkennedy, @smuzaffar can you please review it and eventually sign/assign? Thanks.

cms-bot commands are listed here

@makortel
Copy link
Contributor Author

@cms-sw/generators-l2 @cms-sw/xpog-l2 Could you confirm if GenEventInfoProduct and/or LHEEventProduct data products from 10_6_X (Run 2 UL) AODSIM need to be readable in master for future MiniAOD/NanoAOD productions?

The PR test failures in #43405 (comment) occurred in

  • workflow 158.01 reading /store/relval/CMSSW_10_3_3/RelValHydjetQ_B12_5020GeV_2018_ppReco/GEN-SIM-RECO/PU_103X_upgrade2018_realistic_HI_v11-v1/10000/49F1E805-73F9-AC4C-8E16-27019EE440FC.root
  • pat1 test reading /store/relval/CMSSW_9_2_2/RelValProdTTbar_13/AODSIM/91X_mcRun2_asymptotic_v3-v1/10000/EEB99F74-DA4D-E711-A41C-0025905A48F2.root

@makortel
Copy link
Contributor Author

type root

@cmsbuild cmsbuild added the root label Nov 29, 2023
@vlimant
Copy link
Contributor

vlimant commented Nov 29, 2023

extremely likely, until we do a full Run2 reprocessing and MC (reconstruction from RAW); i.e not any time soon

@makortel
Copy link
Contributor Author

Thanks @vlimant, this is pretty much what I would have guessed (but wanted to make sure).

@makortel
Copy link
Contributor Author

@pcanal We would need an ability to read an old file, and evolve std::auto_ptr<T> (for one specific T=gen::PdfInfo) into something that we can further evolve into std::unique_ptr<T> (or directly to unique_ptr), in some way that avoids explicit use of std::auto_ptr<T> (since it has been removed from the standard, even if libstdc++ still seems to provide the implementation out of the box).

@pcanal
Copy link
Contributor

pcanal commented Nov 29, 2023

For better (or worse, but better in this case), the read rule does not check the type listed in the rules against the incoming class, so all you need is:

namespace Compatibility {
template <typename T>
struct auto_ptr_is_deprecated
{
   ~auto_ptr_is_deprecated() { delete _M_ptr; }
   void release() { _M_ptr = nulltptr; }
   T * get() { return _M_ptr; }
   
   T *_M_ptr = nullptr;
};
} // Compatibility

and

 <ioread sourceClass = "GenEventInfoProduct" version="[11]" targetClass="GenEventInfoProduct" source = "Compatibility::auto_ptr<gen::PdfInfo> pdf_;" target="pdf_">
   <![CDATA[pdf_.reset(onfile.pdf_.get()); onfile.pdf_.release();]]>
 </ioread>

With this style, you need to make 100% sure that the layout of auto_ptr_is_deprecated is exactly the same as the layout implied by the TStreamerInfo for auto_ptr that is stored in the file being read.

Alternatively you can have a class name ::auto_ptr for which you generate a dictionary. In that case the layout does not have to be strictly the same.

@makortel
Copy link
Contributor Author

Thanks @pcanal.

With this style, you need to make 100% sure that the layout of auto_ptr_is_deprecated is exactly the same as the layout implied by the TStreamerInfo for auto_ptr that is stored in the file being read.

What would happen if the layout would not be exactly the same? Would we get an error, or would data be misinterpreted silently?

@pcanal
Copy link
Contributor

pcanal commented Nov 29, 2023

Unfortunately the data would be (possibly) interpreted incorrectly .. silently (i.e. The memory will be allocated and set as described by the TStreamerInfo and for all for intents and purposes reinterpret_cast ed to the stated type.

@Dr15Jones
Copy link
Contributor

Unfortunately, using a different class name (i.e. hepmc_iorule::auto_ptr_is_deprecated) in the iorule source did not work. The JIT still happens and I get the following messages from ROOT

Begin processing the 1st record. Run 1, Event 1, LumiSection 1 on stream 0 at 09-Feb-2024 08:46:10.671 CST
input_line_57:5:26: warning: 'auto_ptr<gen::PdfInfo>' is deprecated: use 'std::unique_ptr' instead [-Wdeprecated-declarations]
         *ret = new std::auto_ptr<gen::PdfInfo>;
                         ^
/data/cmsbld/jenkins/workspace/auto-builds/CMSSW_14_0_0_pre3-el8_amd64_gcc12/build/CMSSW_14_0_0_pre3-build/el8_amd64_gcc12/external/gcc/12.3.1-40d504be6370b5a30e3947a6e575ca28/include/c++/12.3.1/backward/auto_ptr.h:287:7: note: 'auto_ptr<gen::
PdfInfo>' has been explicitly marked deprecated here
    } _GLIBCXX11_DEPRECATED_SUGGEST("std::unique_ptr");
      ^
/data/cmsbld/jenkins/workspace/auto-builds/CMSSW_14_0_0_pre3-el8_amd64_gcc12/build/CMSSW_14_0_0_pre3-build/el8_amd64_gcc12/external/gcc/12.3.1-40d504be6370b5a30e3947a6e575ca28/include/c++/12.3.1/x86_64-redhat-linux-gnu/bits/c++config.h:104:45:
 note: expanded from macro '_GLIBCXX11_DEPRECATED_SUGGEST'
# define _GLIBCXX11_DEPRECATED_SUGGEST(ALT) _GLIBCXX_DEPRECATED_SUGGEST(ALT)
                                            ^
/data/cmsbld/jenkins/workspace/auto-builds/CMSSW_14_0_0_pre3-el8_amd64_gcc12/build/CMSSW_14_0_0_pre3-build/el8_amd64_gcc12/external/gcc/12.3.1-40d504be6370b5a30e3947a6e575ca28/include/c++/12.3.1/x86_64-redhat-linux-gnu/bits/c++config.h:96:19: 
note: expanded from macro '_GLIBCXX_DEPRECATED_SUGGEST'
  __attribute__ ((__deprecated__ ("use '" ALT "' instead")))
                  ^

and the job terminates with the exception

----- Begin Fatal Exception 09-Feb-2024 08:43:29 CST-----------------------                                                                                                                                                                        
An exception of category 'FileReadError' occurred while                                                                                                                                                                                            
   [0] Processing  Event run: 1 lumi: 1 event: 1 stream: 0                                                                                                                                                                                         
   [1] Running path 'e'                                                                                                                                                                                                                            
   [2] Prefetching for module CheckGenEventInfoProduct/'check'                                                                                                                                                                                     
   [3] While reading from source GenEventInfoProduct dummy '' TEST                                                                                                                                                                                 
   [4] Reading branch GenEventInfoProduct_dummy__TEST.                                                                                                                                                                                             
   Additional Info:                                                                                                                                                                                                                                
      [a] Fatal Root Error: @SUB=TClass::BuildRealData                                                                                                                                                                                             
Inspection for auto_ptr<gen::PdfInfo> not supported!                                                                                                                                                                                               
                                                                                                                                                                                                                                                   
----- End Fatal Exception -------------------------------------------------                                                                                                                                                                        

Running in gdb I find the exception comes from

#2  0x00007ffff6e0ab2b in ErrorHandler () from /cvmfs/cms.cern.ch/el8_amd64_gcc12/cms/cmssw/CMSSW_14_0_0_pre3/external/el8_amd64_gcc12/lib/libCore.so                                                                                             
#3  0x00007ffff6d5a3e4 in TObject::Error(char const*, char const*, ...) const () from /cvmfs/cms.cern.ch/el8_amd64_gcc12/cms/cmssw/CMSSW_14_0_0_pre3/external/el8_amd64_gcc12/lib/libCore.so                                                      
#4  0x00007ffff6e27b3f in TClass::BuildRealData(void*, bool) () from /cvmfs/cms.cern.ch/el8_amd64_gcc12/cms/cmssw/CMSSW_14_0_0_pre3/external/el8_amd64_gcc12/lib/libCore.so                                                                       
#5  0x00007ffff71d4068 in TStreamerInfo::BuildOld() () from /cvmfs/cms.cern.ch/el8_amd64_gcc12/cms/cmssw/CMSSW_14_0_0_pre3/external/el8_amd64_gcc12/lib/libRIO.so                                                                                 
#6  0x00007ffff6e19538 in TClass::GetStreamerInfoImpl(int, bool) const () from /cvmfs/cms.cern.ch/el8_amd64_gcc12/cms/cmssw/CMSSW_14_0_0_pre3/external/el8_amd64_gcc12/lib/libCore.so                                                             
#7  0x00007ffff6e1981e in TClass::GetStreamerInfo(int, bool) const () from /cvmfs/cms.cern.ch/el8_amd64_gcc12/cms/cmssw/CMSSW_14_0_0_pre3/external/el8_amd64_gcc12/lib/libCore.so                                                                 
#8  0x00007ffff711503d in TBufferFile::ReadVersion(unsigned int*, unsigned int*, TClass const*) () from /cvmfs/cms.cern.ch/el8_amd64_gcc12/cms/cmssw/CMSSW_14_0_0_pre3/external/el8_amd64_gcc12/lib/libRIO.so                                     
#9  0x00007ffff7116eff in TBufferFile::ReadClassBuffer(TClass const*, void*, TClass const*) () from /cvmfs/cms.cern.ch/el8_amd64_gcc12/cms/cmssw/CMSSW_14_0_0_pre3/external/el8_amd64_gcc12/lib/libRIO.so                                         
#10 0x00007ffff7355f48 in int TStreamerInfo::ReadBuffer<TVirtualArray>(TBuffer&, TVirtualArray const&, TStreamerInfo::TCompInfo* const*, int, int, int, int, int) ()                                                                              
   from /cvmfs/cms.cern.ch/el8_amd64_gcc12/cms/cmssw/CMSSW_14_0_0_pre3/external/el8_amd64_gcc12/lib/libRIO.so
#11 0x00007ffff73571df in int TStreamerInfo::ReadBuffer<char**>(TBuffer&, char** const&, TStreamerInfo::TCompInfo* const*, int, int, int, int, int) ()                                                                                            
   from /cvmfs/cms.cern.ch/el8_amd64_gcc12/cms/cmssw/CMSSW_14_0_0_pre3/external/el8_amd64_gcc12/lib/libRIO.so
#12 0x00007ffff71dd94d in TStreamerInfoActions::GenericReadAction(TBuffer&, void*, TStreamerInfoActions::TConfiguration const*) () from /cvmfs/cms.cern.ch/el8_amd64_gcc12/cms/cmssw/CMSSW_14_0_0_pre3/external/el8_amd64_gcc12/lib/libRIO.so     
#13 0x00007ffff722ec5a in TStreamerInfoActions::UseCache(TBuffer&, void*, TStreamerInfoActions::TConfiguration const*) () from /cvmfs/cms.cern.ch/el8_amd64_gcc12/cms/cmssw/CMSSW_14_0_0_pre3/external/el8_amd64_gcc12/lib/libRIO.so              
#14 0x00007ffff7116eae in TBufferFile::ReadClassBuffer(TClass const*, void*, TClass const*) () from /cvmfs/cms.cern.ch/el8_amd64_gcc12/cms/cmssw/CMSSW_14_0_0_pre3/external/el8_amd64_gcc12/lib/libRIO.so                                         
#15 0x00007ffff735bdcc in int TStreamerInfo::ReadBuffer<char**>(TBuffer&, char** const&, TStreamerInfo::TCompInfo* const*, int, int, int, int, int) ()                                                                                            
   from /cvmfs/cms.cern.ch/el8_amd64_gcc12/cms/cmssw/CMSSW_14_0_0_pre3/external/el8_amd64_gcc12/lib/libRIO.so
#16 0x00007ffff71dd94d in TStreamerInfoActions::GenericReadAction(TBuffer&, void*, TStreamerInfoActions::TConfiguration const*) () from /cvmfs/cms.cern.ch/el8_amd64_gcc12/cms/cmssw/CMSSW_14_0_0_pre3/external/el8_amd64_gcc12/lib/libRIO.so     
#17 0x00007ffff710ebb5 in TBufferFile::ApplySequence(TStreamerInfoActions::TActionSequence const&, void*) () from /cvmfs/cms.cern.ch/el8_amd64_gcc12/cms/cmssw/CMSSW_14_0_0_pre3/external/el8_amd64_gcc12/lib/libRIO.so                           
#18 0x00007ffff7872b87 in TBranchElement::ReadLeavesMember(TBuffer&) () from /cvmfs/cms.cern.ch/el8_amd64_gcc12/cms/cmssw/CMSSW_14_0_0_pre3/external/el8_amd64_gcc12/lib/libTree.so                                                               
#19 0x00007ffff786b429 in TBranch::GetEntry(long long, int) () from /cvmfs/cms.cern.ch/el8_amd64_gcc12/cms/cmssw/CMSSW_14_0_0_pre3/external/el8_amd64_gcc12/lib/libTree.so                                                                        
#20 0x00007ffff787dd44 in TBranchElement::GetEntry(long long, int) () from /cvmfs/cms.cern.ch/el8_amd64_gcc12/cms/cmssw/CMSSW_14_0_0_pre3/external/el8_amd64_gcc12/lib/libTree.so                                                                 
#21 0x00007ffff787dcfd in TBranchElement::GetEntry(long long, int) () from /cvmfs/cms.cern.ch/el8_amd64_gcc12/cms/cmssw/CMSSW_14_0_0_pre3/external/el8_amd64_gcc12/lib/libTree.so                                                                 
#22 0x00007fffcccd685c in edm::RootTree::getEntry(TBranch*, long long) const () from /cvmfs/cms.cern.ch/el8_amd64_gcc12/cms/cmssw/CMSSW_14_0_0_pre3/lib/el8_amd64_gcc12/pluginIOPoolInput.so                                                      
#23 0x00007fffcccb739c in edm::RootDelayedReader::getProduct_(edm::BranchID const&, edm::EDProductGetter const*) () from /cvmfs/cms.cern.ch/el8_amd64_gcc12/cms/cmssw/CMSSW_14_0_0_pre3/lib/el8_amd64_gcc12/pluginIOPoolInput.so                  

@Dr15Jones
Copy link
Contributor

Using ::auto_ptr also does not work as at run time I get the error

Begin processing the 1st record. Run 1, Event 1, LumiSection 1 on stream 0 at 09-Feb-2024 09:05:49.444 CST                                                                                                                                         
input_line_57:5:21: error: reference to 'auto_ptr' is ambiguous                                                                                                                                                                                    
         *ret = new auto_ptr<gen::PdfInfo>;                                                                                                                                                                                                        
                    ^                                                                                                                                                                                                                              
SimDataFormatsGeneratorProducts_xr dictionary payload:102:8: note: candidate found by name lookup is 'auto_ptr'                                                                                                                                    
struct auto_ptr
       ^
/data/cmsbld/jenkins/workspace/auto-builds/CMSSW_14_0_0_pre3-el8_amd64_gcc12/build/CMSSW_14_0_0_pre3-build/el8_amd64_gcc12/external/gcc/12.3.1-40d504be6370b5a30e3947a6e575ca28/include/c++/12.3.1/bits/unique_ptr.h:64:28: note: candidate found by name lookup is 'std::auto_ptr'
  template<typename> class auto_ptr;
                           ^
input_line_57:8:21: error: reference to 'auto_ptr' is ambiguous
         *ret = new auto_ptr<gen::PdfInfo>[nary];
                    ^
SimDataFormatsGeneratorProducts_xr dictionary payload:102:8: note: candidate found by name lookup is 'auto_ptr'
struct auto_ptr
       ^
/data/cmsbld/jenkins/workspace/auto-builds/CMSSW_14_0_0_pre3-el8_amd64_gcc12/build/CMSSW_14_0_0_pre3-build/el8_amd64_gcc12/external/gcc/12.3.1-40d504be6370b5a30e3947a6e575ca28/include/c++/12.3.1/bits/unique_ptr.h:64:28: note: candidate found by name lookup is 'std::auto_ptr'
  template<typename> class auto_ptr;
                           ^
input_line_57:13:29: error: reference to 'auto_ptr' is ambiguous
         *ret = new (arena) auto_ptr<gen::PdfInfo>;
                            ^
SimDataFormatsGeneratorProducts_xr dictionary payload:102:8: note: candidate found by name lookup is 'auto_ptr'
struct auto_ptr
       ^
/data/cmsbld/jenkins/workspace/auto-builds/CMSSW_14_0_0_pre3-el8_amd64_gcc12/build/CMSSW_14_0_0_pre3-build/el8_amd64_gcc12/external/gcc/12.3.1-40d504be6370b5a30e3947a6e575ca28/include/c++/12.3.1/bits/unique_ptr.h:64:28: note: candidate found by name lookup is 'std::auto_ptr'
  template<typename> class auto_ptr;
                           ^
input_line_57:16:29: error: reference to 'auto_ptr' is ambiguous
         *ret = new (arena) auto_ptr<gen::PdfInfo>[nary];
                            ^
SimDataFormatsGeneratorProducts_xr dictionary payload:102:8: note: candidate found by name lookup is 'auto_ptr'
struct auto_ptr
       ^
/data/cmsbld/jenkins/workspace/auto-builds/CMSSW_14_0_0_pre3-el8_amd64_gcc12/build/CMSSW_14_0_0_pre3-build/el8_amd64_gcc12/external/gcc/12.3.1-40d504be6370b5a30e3947a6e575ca28/include/c++/12.3.1/bits/unique_ptr.h:64:28: note: candidate found b
y name lookup is 'std::auto_ptr'
  template<typename> class auto_ptr;

----- Begin Fatal Exception 09-Feb-2024 09:05:49 CST-----------------------
An exception of category 'FileReadError' occurred while
   [0] Processing  Event run: 1 lumi: 1 event: 1 stream: 0
   [1] Running path 'e'
   [2] Prefetching for module CheckGenEventInfoProduct/'check'
   [3] While reading from source GenEventInfoProduct dummy '' TEST
   [4] Reading branch GenEventInfoProduct_dummy__TEST.
   Additional Info:
      [a] Fatal Root Error: @SUB=TClingCallFunc::make_ctor_wrapper
Failed to compile
  ==== SOURCE BEGIN ====
__attribute__((used)) extern "C" void __ctor_2(void** ret, void* arena, unsigned long nary)
{
   if (!arena) {
      if (!nary) {
         *ret = new auto_ptr<gen::PdfInfo>;
      }
      else {
         *ret = new auto_ptr<gen::PdfInfo>[nary];
      }
   }
   else {
      if (!nary) {
         *ret = new (arena) auto_ptr<gen::PdfInfo>;
      }
      else {
         *ret = new (arena) auto_ptr<gen::PdfInfo>[nary];
      }
   }
}

  ==== SOURCE END ====

----- End Fatal Exception -------------------------------------------------

When I had ::auto_ptr<gen::PdfInfo> declared in the classes_def.xml file, scram always failed as genreflex kept issuing a warning that the type was unused. My attempts to force the type to exist all failed.

@makortel
Copy link
Contributor Author

makortel commented Feb 9, 2024

@cms-sw/xpog-l2 @cms-sw/generators-l2 Given the discovery that even the present IO rule to convert the auto_ptr to unique_ptr does not work properly for split input like AOD (#43923), I'm now wondering if the 10_6_X UL AODSIM has already been used as an input for MiniAOD production in release newer than 11_0_0?

@vlimant
Copy link
Contributor

vlimant commented Feb 15, 2024

10.6 AOD/AODSIM has not been used as input to PAT/MINI => NANO workflow in recent releases indeed ; although this is the general plan we target (instead of just redoing NANO on top of 10.6 MINI)

@vlimant
Copy link
Contributor

vlimant commented Feb 15, 2024

assign xpog

@cmsbuild
Copy link
Contributor

New categories assigned: xpog

@vlimant,@hqucms you have been requested to review this Pull request/Issue and eventually sign? Thanks

@vlimant
Copy link
Contributor

vlimant commented Feb 15, 2024

assign generators

@cmsbuild
Copy link
Contributor

New categories assigned: generators

@alberto-sanchez,@bbilin,@GurpreetSinghChahal,@mkirsano,@menglu21,@SiewYan you have been requested to review this Pull request/Issue and eventually sign? Thanks

@makortel
Copy link
Contributor Author

Thanks @vlimant. If the gen::PdfInfo is desired to be read from the 10_6_X AODSIM files, we (core) want to understand deeply where it is used, and what generators/samples include the object (or conversely, what would be impact if this object would just be ignored). In this case it would also be great to have a test to be run in IBs that fails when the gen::PdfInfo is expected to be present, but is not.

Then, we need ROOT team to fix #43923, and give us further guidance on how to remove the explicit use of std::auto_ptr in the IO read rule.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants