cmssw icon indicating copy to clipboard operation
cmssw copied to clipboard

Failure in RecoPPSLocalNewT2 unit test

Open makortel opened this issue 1 year ago • 28 comments

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.

makortel avatar May 30 '24 13:05 makortel

cms-bot internal usage

cmsbuild avatar May 30 '24 13:05 cmsbuild

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

cmsbuild avatar May 30 '24 13:05 cmsbuild

assign RecoPPS/Local

makortel avatar May 30 '24 13:05 makortel

New categories assigned: reconstruction

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

cmsbuild avatar May 30 '24 13:05 cmsbuild

@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?

makortel avatar May 30 '24 13:05 makortel

type ctpps

jfernan2 avatar May 30 '24 17:05 jfernan2

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

makortel avatar Jun 04 '24 22:06 makortel

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 avatar Jun 05 '24 07:06 grzanka

@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 ?

grzanka avatar Jun 05 '24 08:06 grzanka

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?

makortel avatar Jun 05 '24 13:06 makortel

@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

smuzaffar avatar Jun 05 '24 13:06 smuzaffar

@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

grzanka avatar Jun 05 '24 13:06 grzanka

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.

smuzaffar avatar Jun 05 '24 13:06 smuzaffar

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 avatar Jun 05 '24 13:06 grzanka

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'),
        )
)

grzanka avatar Jun 05 '24 14:06 grzanka

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

makortel avatar Jun 05 '24 14:06 makortel

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

smuzaffar avatar Jun 05 '24 14:06 smuzaffar

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)

makortel avatar Jun 05 '24 14:06 makortel

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

smuzaffar avatar Jun 05 '24 14:06 smuzaffar

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).

makortel avatar Jun 05 '24 14:06 makortel

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

smuzaffar avatar Jun 05 '24 14:06 smuzaffar

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 ?

grzanka avatar Jun 05 '24 14:06 grzanka

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.

smuzaffar avatar Jun 05 '24 14:06 smuzaffar

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 avatar Jun 05 '24 14:06 grzanka

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

grzanka avatar Jun 07 '24 10:06 grzanka

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

iarspider avatar Jun 07 '24 11:06 iarspider

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.

antoniovilela avatar Jun 07 '24 12:06 antoniovilela

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

grzanka avatar Jun 11 '24 13:06 grzanka

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

mandrenguyen avatar Jul 07 '24 22:07 mandrenguyen

This issue is fully signed and ready to be closed.

cmsbuild avatar Jul 07 '24 22:07 cmsbuild