cms-sw / cmssw

CMS Offline Software
http://cms-sw.github.io/
Apache License 2.0
1.06k stars 4.25k forks source link

Failure in RecoPPSLocalNewT2 unit test #45101

Closed makortel closed 1 week ago

makortel commented 1 month ago

The unit test RecoPPSLocalNewT2 fails in CMSSW_14_1_X_2024-05-30-1100 as

===== Test "RecoPPSLocalNewT2" ====
30-May-2024 13:09:44 CEST  Initiating request to open file http://cmsrep.cern.ch/cmssw/download/data/RecoPPS/Local/V1/run364983_ls0001_streamA_StorageManager.dat
30-May-2024 13:09:44 CEST  Successfully opened file http://cmsrep.cern.ch/cmssw/download/data/RecoPPS/Local/V1/run364983_ls0001_streamA_StorageManager.dat
30-May-2024 13:09:44 CEST  Closed file http://cmsrep.cern.ch/cmssw/download/data/RecoPPS/Local/V1/run364983_ls0001_streamA_StorageManager.dat
----- Begin Fatal Exception 30-May-2024 13:09:44 CEST-----------------------
An exception of category 'FatalRootError' occurred while
   [0] Constructing the EventProcessor
   [1] Constructing input source of type NewEventStreamFileReader
   Additional Info:
      [a] Fatal Root Error: @SUB=TBufferFile::ReadClassBuffer
Could not find the StreamerInfo for version 11 of the class edm::SendJobHeader, object skipped at offset 33

----- End Fatal Exception -------------------------------------------------
Failure using /data/cmsbld/jenkins/workspace/ib-run-qa/CMSSW_14_1_X_2024-05-30-1100/src/RecoPPS/Local/test/totemT2NewDigi_reco_cfg.py: status 86

---> test RecoPPSLocalNewT2 had ERRORS

https://cmssdt.cern.ch/SDT/cgi-bin/logreader/el8_amd64_gcc12/CMSSW_14_1_X_2024-05-30-1100/unitTestLogs/RecoPPS/Local#/

(noted also in https://github.com/cms-sw/cmssw/pull/44892#issuecomment-2139443543)

The failure is a consequence of the merge of https://github.com/cms-sw/cmssw/pull/44892, but in this case it is the test that needs an update.

cmsbuild commented 1 month ago

cms-bot internal usage

cmsbuild commented 1 month ago

A new Issue was created by @makortel.

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

cms-bot commands are listed here

makortel commented 1 month ago

assign RecoPPS/Local

cmsbuild commented 1 month ago

New categories assigned: reconstruction

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

makortel commented 1 month ago

@cms-sw/ctpps-dpg-l2 The test fails because the streamer format was changed. Does this test really need to read a streamer file, or could it read a root file?

jfernan2 commented 1 month ago

type ctpps

makortel commented 1 month ago

Could someone look into this? (or should we look into disabling the test?)

grzanka commented 1 month ago

Could someone look into this? (or should we look into disabling the test?)

I've investigated the test a bit. It seems that we can convert the input streamer file into RAW ROOT format and run the problematic test on it. If everything goes well with the conversion I'll put converted file on CERNBOX and make the PR with fixes (once the file is available in the area where testing machinery can access it).

grzanka commented 1 month ago

@makortel I've put the converted RAW file here: https://cernbox.cern.ch/s/OD136wlNpQfH4X8 , would it be possible to upload it somewhere where the Jenkins can access it ?

makortel commented 1 month ago

Thanks @grzanka! To me it would make sense to place the new file where the present file is, whose location is defined in https://github.com/cms-data/RecoPPS-Local , but I don't know how to get the file in http://cmsrep.cern.ch/cmssw/download . @smuzaffar could you help?

smuzaffar commented 1 month ago

@grzanka , if the new file is less than 50MB then I would suggest to open a PR and add it to https://github.com/cms-data/RecoPPS-Local . If it ia over 50MB then please copy it to your afs public directory

grzanka commented 1 month ago

@grzanka , if the new file is less than 50MB then I would suggest to open a PR and add it to https://github.com/cms-data/RecoPPS-Local . If it ia over 50MB then please copy it to your afs public directory

The file is ~77MB, I've copied it to /afs/cern.ch/user/g/grzankal/public

smuzaffar commented 1 month ago

thanks @grzanka , so do we need to keep both

files in RecoPPS/Local or just the new one is enough?

By the way, we should not use http://cmsrep.cern.ch/cmssw/download/data/RecoPPS/Local to download this file. This file is path of data-RecoPPS-Local external and should be accessed/opened using FileInPath RecoPPS/Local/data/run364983_ls0001_streamA_StorageManager.dat or RecoPPS/Local/data/run364983_ls0001_raw.root.

grzanka commented 1 month ago

thanks @grzanka , so do we need to keep both

  • existing run364983_ls0001_streamA_StorageManager.dat
  • new run364983_ls0001_raw.root

files in RecoPPS/Local or just the new one is enough?

By the way, we should not use http://cmsrep.cern.ch/cmssw/download/data/RecoPPS/Local to download this file. This file is path of data-RecoPPS-Local external and should be accessed/opened using FileInPath RecoPPS/Local/data/run364983_ls0001_streamA_StorageManager.dat or RecoPPS/Local/data/run364983_ls0001_raw.root.

For the moment we would prefer to keep both files. Thank you for the hints with the paths, I will now prepare a PR with fix

grzanka commented 1 month ago

By the way, we should not use http://cmsrep.cern.ch/cmssw/download/data/RecoPPS/Local to download this file. This file is path of data-RecoPPS-Local external and should be accessed/opened using FileInPath RecoPPS/Local/data/run364983_ls0001_streamA_StorageManager.dat or RecoPPS/Local/data/run364983_ls0001_raw.root.

How to use FileInPath in context of cms.Source('PoolSource' ? The code below doesn't make much sense....

process.source = cms.Source('PoolSource',
    fileNames =  cms.untracked.vstring(
        cms.untracked.FileInPath('RecoPPS/Local/data/run364983_ls0001_streamA_StorageManager.dat'),
        )
)
makortel commented 1 month ago

How to use FileInPath in context of cms.Source('PoolSource' ? The code below doesn't make much sense....

You indeed can't. However, there is a utility that resolves the search in python and returns vstring, see https://github.com/cms-sw/cmssw/blob/a9682e8ca89d4942c6676bf8c5f0d2c6a70abb0e/FWCore/Modules/test/testBunchCrossingFilter.py#L27 https://github.com/cms-sw/cmssw/blob/a9682e8ca89d4942c6676bf8c5f0d2c6a70abb0e/FWCore/Modules/test/testBunchCrossingFilter.py#L42-L44 for an example

smuzaffar commented 1 month ago

https://github.com/cms-data/RecoPPS-Local/pull/2 is now open, cmssw PR should use this for testing

makortel commented 1 month ago

For the moment we would prefer to keep both files.

If the old file would be removed, the unit test would start to fail in old release cycles. Which reminds me, the fix in the unit test needs to be backported to 14_0_X too (or merging https://github.com/cms-sw/cmssw/pull/44978 will cause the test to fail there)

smuzaffar commented 1 month ago

I meant, do we need to drop the old file from data-RecoPPS-Local package? i.e. it will not be removed from the cmsrep.cern.ch download area but from the package we distribute on cvmfs. Old releases will keep one accessing it from cmsrep

makortel commented 1 month ago

I meant, do we need to drop the old file from data-RecoPPS-Local package? i.e. it will not be removed from the cmsrep.cern.ch download area but from the package we distribute on cvmfs. Old releases will keep one accessing it from cmsrep

Ah, then I'd be fine with removing the old file from data-RecoPPS-Local package. Do I understand correctly the benefit would be that the new releases would no longer distribute the old file (that would be unnecessary there).

smuzaffar commented 1 month ago

yes, new tag/version of data-RecoPPS-Local external will only have one file (run364983_ls0001_raw.root) .

grzanka commented 1 month ago

I've made the PR with necessary fixes: https://github.com/cms-sw/cmssw/pull/45144 Unfortunately I wasn't able to make full test locally:

[grzankal@lxplus952 test]$ cmsRun totemT2NewDigi_reco_cfg.py
----- Begin Fatal Exception 05-Jun-2024 16:26:09 CEST-----------------------
An exception of category 'ConfigFileReadError' occurred while
   [0] Processing the python configuration file named totemT2NewDigi_reco_cfg.py
Exception Message:
 unknown python problem occurred.
OSError: No such file or directory 'RecoPPS/Local/data/run364983_ls0001_raw.root' in the CMSSW_SEARCH_PATH

At:
  /cvmfs/cms-ib.cern.ch/sw/x86_64/week0/el9_amd64_gcc12/cms/cmssw/CMSSW_14_1_X_2024-06-02-2300/src/FWCore/ParameterSet/python/pfnInPath.py(10): pfnInPath
  /cvmfs/cms-ib.cern.ch/sw/x86_64/week0/el9_amd64_gcc12/cms/cmssw/CMSSW_14_1_X_2024-06-02-2300/src/FWCore/ParameterSet/python/pfnInPath.py(16): <genexpr>

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

I suspect that would require a new release or the PR being merged. Can you run necessary Jenkins tests on the PR ?

smuzaffar commented 1 month ago

Jenkins tests are running for https://github.com/cms-sw/cmssw/pull/45144 . In order to test it locally you can just copy run364983_ls0001_raw.root file in your <CMSSW-dev-area>/src/RecoPPS/Local/data/ path and run the test.

grzanka commented 1 month ago

Jenkins tests are running for #45144 . In order to test it locally you can just copy run364983_ls0001_raw.root file in your <CMSSW-dev-area>/src/RecoPPS/Local/data/ path and run the test.

Thanks, it worked !

grzanka commented 1 month ago

It seems the tests on https://github.com/cms-sw/cmssw/pull/45144 succeeded, what would be the next step ?

iarspider commented 1 month ago

It will be reviewed by corresponding L1 (reconstruction in this case), then by ORPs, which will then merge it.

antoniovilela commented 1 month ago

Will we need the backport to 14_0_X? The streamer file change will be merged soon in 14_0_X. Remember to make the cmsdist backport, to point to the updated cms-data tag.

grzanka commented 1 month ago

Will we need the backport to 14_0_X? The streamer file change will be merged soon in 14_0_X. Remember to make the cmsdist backport, to point to the updated cms-data tag.

As requested I've made a backport https://github.com/cms-sw/cmssw/pull/45192

mandrenguyen commented 1 week ago

+1 Addressed by https://github.com/cms-sw/cmssw/pull/45192

cmsbuild commented 1 week ago

This issue is fully signed and ready to be closed.