Remove code related to legacy modules and SharedResources #44271
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
cms-bot internal usage |
|
+code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-44271/39289
|
|
A new Pull Request was created by @wddgit for master. It involves the following packages:
@smuzaffar, @makortel, @Dr15Jones, @cmsbuild can you please review it and eventually sign? Thanks. cms-bot commands are listed here |
|
please test |
|
+1 Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-e6bf4e/37825/summary.html Comparison SummarySummary:
|
|
Comparisons show the usual
and a large number of differences (8237) in 9.0, likely related to #34448 |
|
@cmsbuild, please test I want to see if the behavior on 9.0 reproduces (likely not) |
|
+1 Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-e6bf4e/37839/summary.html Comparison SummarySummary:
|
|
Looks like the 9.0 behavior did reproduce. Although I can't see how it could be related the changes in the PR. |
|
The baseline IB was the same. IIRC that was the situation also earlier when this rare non-reproducibility was observed (#34448). Let's try once more. |
|
@cmsbuild, please test |
|
I ran 9.0 on CMSSW_14_1_X_2024-03-04-1100, and again on CMSSW_14_1_X_2024-03-04-1100 with this PR recompiled locally (on slc7).
library=libDetectorDescriptionCore.so
+library=libDataFormatsCLHEP.so
library=libDetectorDescriptionDDCMS.so
library=libCondFormatsGeometryObjects.so
-library=libDataFormatsCLHEP.so
library=libDataFormatsEcalDetId.so
...
library=libCondCorePopCon.so
+library=libGeometryVeryForwardGeometryBuilder.so
library=libCalibPPSESProducers.so
library=libCondCoreDBOutputService.so
-library=libGeometryVeryForwardGeometryBuilder.so
library=libCondFormatsPPSObjects.so
-library=libDataFormatsCTPPSDetId.so
library=libCalibPPSAlignmentGlobal.so
+library=libDataFormatsCTPPSDetId.so
library=libUtilitiesXerces.so
...
library=libDataFormatsTrajectoryState.so
+library=libSimDataFormatsCaloHit.so
library=libDataFormatsForwardDetId.so
library=libDataFormatsHcalDetId.so
library=libDataFormatsSiPixelDetId.so
library=libDataFormatsSiStripDetId.so
-library=libSimDataFormatsCaloHit.so
library=libDataFormatsPhase2TrackerDigi.so
...
library=libCommonToolsUtilAlgos.so
-library=libCondFormatsCastorObjects.so
+library=libCommonToolsUtils.so
library=libDataFormatsHcalDigi.so
+library=libCondFormatsCastorObjects.so
library=libCalibFormatsCaloObjects.so
-library=libCommonToolsUtils.so
library=libboost_regex.so.1.80.0
...
library=libCondFormatsSiStripObjects.so
-library=libDataFormatsSiPixelDigi.so
-library=libDataFormatsSiStripCommon.so
library=libHeterogeneousCoreAlpakaCore.so
-library=libDataFormatsFEDRawData.so
library=libCondFormatsPhysicsToolsObjects.so
+library=libDataFormatsSiStripCommon.so
+library=libDataFormatsSiPixelDigi.so
+library=libDataFormatsFEDRawData.so
library=libCalibFormatsSiPixelObjects.so
...
library=libGeneratorInterfaceExternalDecays.so
-library=libGeneratorInterfacePythia6Interface.so
library=libGeneratorInterfaceTauolaInterface.so
-library=libGeneratorInterfaceCore.so
library=libGeneratorInterfacePartonShowerVeto.so
+library=libGeneratorInterfacePythia6Interface.so
+library=libGeneratorInterfaceAlpgenInterface.so
+library=libGeneratorInterfaceCore.so
library=libGeneratorInterfaceEvtGenInterface.so
-library=libGeneratorInterfaceLHEInterface.so
library=libGeneratorInterfacePhotosInterface.so
-library=libGeneratorInterfaceAlpgenInterface.so
+library=libGeneratorInterfaceLHEInterface.so
library=libSimDataFormatsGeneratorProducts.so
...
library=libIOPoolStreamer.so
-library=libDataFormatsL1ScoutingRawData.so
library=libDataFormatsLuminosity.so
+library=libDataFormatsL1ScoutingRawData.so
library=libDataFormatsTCDS.so
...
library=libDataFormatsHGCRecHit.so
-library=libPhysicsToolsTensorFlow.so
library=libSimDataFormatsCaloTest.so
-library=libSimDataFormatsEcalTestBeam.so
library=libSimDataFormatsTrackingHit.so
library=libSimDataFormatsValidationFormats.so
+library=libPhysicsToolsTensorFlow.so
library=libSimG4CoreWatcher.so
+library=libSimDataFormatsEcalTestBeam.so
library=libTBDataFormatsEcalTBObjects.so |
Already the first event shows a difference GenVertex: -7 ID: 0 (X,cT):0
- I: 1 9 37 -5.65e+01,-8.86e+00,+1.31e+02,+2.59e+02 3 -7
- O: 3 12 -15 +2.20e+01,-2.10e+01,-5.18e+01,+6.01e+01 3 -9
- 13 16 -7.85e+01,+1.21e+01,+1.83e+02,+1.99e+02 3 -10
- 16 37 -5.65e+01,-8.86e+00,+1.31e+02,+2.59e+02 2
+ I: 1 9 -37 -3.08e+02,+1.62e+02,+2.63e+02,+4.87e+02 3 -7
+ O: 3 12 15 -3.43e+00,+7.10e+01,-5.04e+00,+7.12e+01 3 -9
+ 13 -16 -3.04e+02,+9.12e+01,+2.68e+02,+4.16e+02 3 -10
+ 16 -37 -3.08e+02,+1.62e+02,+2.63e+02,+4.87e+02 2 |
|
So if you run the job again (either with or without the PR) do you get the same GenVertex results or does it change yet again? |
|
I recompiled my developer area with the IB tag (i.e. the same packages as checked out with Also the library load order is the same (with the exception of |
|
The GenVertex printout comes from |
|
I added changes done in this PR in a few steps, and but that didn't reproduce the difference. I then recompiled the PR again, and again didn't reproduce the difference. It seems at least the library load order can be crossed out. |
|
@cmsbuild, please test Let's do one more time |
|
+1 Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-e6bf4e/37997/summary.html Comparison SummarySummary:
|
|
Now the 9.0 differences are gone here too. What remains are the usual |
|
+core |
cmsbuild
added
core-approved
fully-signed
and removed
core-pending
pending-signatures
labels
|
This pull request is fully signed and it will be integrated in one of the next master IBs (tests are also fine). This pull request will now be reviewed by the release team before it's merged. @rappoccio, @antoniovilela, @sextonkennedy (and backports should be raised in the release meeting by the corresponding L2) |
|
This goes totally off-topic, but running valgrind on the GEN-SIM step of the 9.0 shows many and so random behavior is not really surprising (maybe the surprise is that it doesn't happen more often). This could be related to the discussions in #35082, #35251, #43526 (maybe we should finally address #43526 (comment)) |
Now reported in #44371 |
|
+1 |
This was referenced Mar 13, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
PR description:
Remove code related to SharedResources and legacy type modules. We don't need this anymore as there aren't any legacy modules anymore. It simplifies the code and makes maintenance easier.
This also removes the overload of the usesResource function with no arguments. It was possible to call this from a "one" type module and then it would behave like a legacy module. We don't want to preserve that ability. Nothing in CMSSW is currently using it. With this change, you will be required to supply a name for usesResource as an argument.
In a couple places, there is an if-else block and only one conditional path remains after the change. That block remains identical except for indentation.
PR validation:
Mostly this is deletions. It builds successfully even after checkdeps is run. Framework unit tests pass.