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

Revert "PPS: rename directories with misleading names" #29093

Merged
merged 1 commit into from Mar 4, 2020

Conversation

silviodonato
Copy link
Contributor

Reverts #29037

@cmsbuild
Copy link
Contributor

cmsbuild commented Mar 4, 2020

The code-checks are being triggered in jenkins.

@cmsbuild
Copy link
Contributor

cmsbuild commented Mar 4, 2020

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-29093/13986

@cmsbuild
Copy link
Contributor

cmsbuild commented Mar 4, 2020

A new Pull Request was created by @silviodonato (Silvio Donato) for master.

It involves the following packages:

CalibPPS/ESProducers
CondCore/CTPPSPlugins
CondCore/Utilities
CondFormats/CTPPSReadoutObjects
CondFormats/PPSObjects
CondTools/CTPPS
Configuration/EventContent
Configuration/StandardSequences
DQM/CTPPS
DQM/Integration
EventFilter/CTPPSRawToDigi
Geometry/VeryForwardGeometry
Geometry/VeryForwardGeometryBuilder
IOMC/EventVertexGenerators
PhysicsTools/PatAlgos
RecoCTPPS/Configuration
RecoCTPPS/PixelLocal
RecoCTPPS/ProtonReconstruction
RecoCTPPS/TotemRPLocal
RecoPPS/Configuration
SimPPS/PPSPixelDigiProducer
SimPPS/RPDigiProducer
Validation/CTPPS

The following packages do not have a category, yet:

CondFormats/PPSObjects
RecoPPS/Configuration
Please create a PR for https://github.com/cms-sw/cms-bot/blob/master/categories_map.py to assign category

@andrius-k, @schneiml, @ianna, @fioriNTU, @tlampen, @pohsun, @santocch, @perrotta, @civanch, @makortel, @cmsbuild, @davidlange6, @Dr15Jones, @cvuosalo, @mdhildreth, @jfernan2, @tocheng, @slava77, @ggovi, @fabiocos, @kmaeshima, @christopheralanwest, @silviodonato, @franzoni can you please review it and eventually sign? Thanks.
@rappoccio, @gouskos, @felicepantaleo, @forthommel, @hatakeyamak, @emilbols, @Martin-Grunewald, @ahinzmann, @threus, @peruzzim, @seemasharmafnal, @mmarionncern, @makortel, @smoortga, @jdolen, @ferencek, @GiacomoSguazzoni, @rovere, @VinInn, @jdamgov, @nhanvtran, @gkasieczka, @tocheng, @schoef, @mmusich, @dgulhan, @clelange, @batinkov, @riga, @jan-kaspar, @JyothsnaKomaragiri, @mverzett, @lecriste, @gpetruc, @mariadalfonso, @andrzejnovak this is something you requested to watch as well.
@davidlange6, @silviodonato, @fabiocos you are the release manager for this.

cms-bot commands are listed here

@perrotta
Copy link
Contributor

perrotta commented Mar 4, 2020

@silviodonato : are you planning to revert #29093?

As far as I can see, there are a few unit tests which fail with some related message, e.g.:

> Warning in <TInterpreter::ReadRootmapFile>: class  PPSTimingCalibration found in libCondFormatsCTPPSReadoutObjects.so  is already in libCondFormatsPPSObjects.so 
> Warning in <TInterpreter::ReadRootmapFile>: class  PPSTimingCalibration::Key found in libCondFormatsCTPPSReadoutObjects.so  is already in libCondFormatsPPSObjects.so 
> Warning in <TInterpreter::ReadRootmapFile>: class  pair<PPSTimingCalibration::Key,pair<double,double> > found in libCondFormatsCTPPSReadoutObjects.so  is already in libCondFormatsPPSObjects.so 

The CondFormats/CTPPSReadoutObjects package does not exist any more after that PR, and so should its library.

The issue must be related to what described by @Dr15Jones in #29037 (comment)

@mrodozov
Copy link
Contributor

mrodozov commented Mar 4, 2020

is this related to the failing tests ?

@silviodonato
Copy link
Contributor Author

merge

@silviodonato
Copy link
Contributor Author

@perrotta, yes, I reverted #29037 because there were many failures in the IB tests.
I would postpone #29037 after pre4.

@perrotta
Copy link
Contributor

perrotta commented Mar 4, 2020

@perrotta, yes, I reverted #29037 because there were many failures in the IB tests.
I would postpone #29037 after pre4.

You will postpone the same issue to pre5...

@silviodonato
Copy link
Contributor Author

@perrotta, yes, I reverted #29037 because there were many failures in the IB tests.
I would postpone #29037 after pre4.

You will postpone the same issue to pre5...

I prefer to have more time to understand and fix the problem, without the pressing of the pre4 deadline.

@Dr15Jones
Copy link
Contributor

@pcanal could you comment about the new ROOT warning? Our guess is ROOT is checking all directories listed in LD_LIBRARY_PATH for root map files even if a given library is found multiple directories in the environment variable.

@pcanal
Copy link
Contributor

pcanal commented Mar 4, 2020

Yes, ROOT looks in all directories in LD_LIBRARY_PATH for rootmap files.

even if a given library is found multiple directories in the environment variable.

There seems to be protection against that in the code ... or more exactly the duplicate avoidance is base in the rootmap filename.

Note that in the example above it does complains (3 different time) about 2 distincts libraries.

@cvuosalo
Copy link
Contributor

+1

1 similar comment
@jfernan2
Copy link
Contributor

+1

@cmsbuild
Copy link
Contributor

cmsbuild commented Apr 29, 2020

The tests are being triggered in jenkins.
https://cmssdt.cern.ch/jenkins/job/ib-run-pr-tests/5896/console Started: 2020/04/29 10:02

@cmsbuild
Copy link
Contributor

+1
Tested at: 11361c6
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-581525/5896/summary.html
CMSSW: CMSSW_11_1_X_2020-04-28-2300
SCRAM_ARCH: slc7_amd64_gcc820

@cmsbuild
Copy link
Contributor

Comparison job queued.

@cmsbuild
Copy link
Contributor

Comparison is ready
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-581525/5896/summary.html

Comparison Summary:

  • No significant changes to the logs found
  • Reco comparison results: 0 differences found in the comparisons
  • DQMHistoTests: Total files compared: 34
  • DQMHistoTests: Total histograms compared: 2696435
  • DQMHistoTests: Total failures: 23
  • DQMHistoTests: Total nulls: 0
  • DQMHistoTests: Total successes: 2696093
  • DQMHistoTests: Total skipped: 319
  • DQMHistoTests: Total Missing objects: 0
  • DQMHistoSizes: Histogram memory added: 0.0 KiB( 33 files compared)
  • Checked 147 log files, 16 edm output root files, 34 DQM output files

@smuzaffar smuzaffar deleted the revert-29037-pps_rename branch June 13, 2020 17:28
@jfernan2
Copy link
Contributor

+1

@cmsbuild
Copy link
Contributor

Pull request #29093 was updated. @perrotta, @rappoccio, @vlimant, @consuegs, @mdhildreth, @saumyaphor4252, @civanch, @simonepigazzini, @antoniovilela, @francescobrivio, @fabiocos, @davidlange6 can you please check and sign again.

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