Improve PortableHost{Collection,Object} dictionary declarations
#48824
Conversation
|
cms-bot internal usage |
|
+code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-48824/45942 |
|
A new Pull Request was created by @makortel for master. It involves the following packages:
@cmsbuild, @fwyzard, @makortel can you please review it and eventually sign? Thanks. cms-bot commands are listed here |
|
@cmsbuild, please test |
|
+1 Size: This PR adds an extra 20KB to repository Comparison SummarySummary:
|
|
Milestone for this pull request has been moved to CMSSW_16_0_X. Please open a backport if it should also go in to CMSSW_15_1_X. |
|
@fwyzard Given
regardless of #49458 we should probably revive this PR and go systematically through all dictionary definitions of I have a vague recollection though there was some detail I was not happy with, but I can't remember what it was. |
|
I must admit I actually didn't remember about this PR :-( |
makortel
force-pushed
the
portableCollectionDictionary
branch
from
cb4f965 to
d001897
Compare
makortel
changed the title
PortableHost{Collection,Object} dictionary declarationsPortableHost{Collection,Object} dictionary declarations
|
I rebased the branch to 16_0_0_pre4, updated the documentation for the necessity to include the |
| <!-- TODO: we should find a better way than replicating the class versions and checksums --> | ||
| <class name="BeamSpotHost::Product" ClassVersion="3"> | ||
| <version ClassVersion="3" checksum="280341519"/> | ||
| </class> | ||
|
|
||
| <class name="BeamSpotPOD" ClassVersion="3"> | ||
| <version ClassVersion="3" checksum="280341519"/> | ||
| </class> |
There was a problem hiding this comment.
@pcanal I think we discussed about the apparent need to define the class versions and checksums twice (the BeamSpotHost::Product is an alias for BeamSpotPOD). Or, I didn't manage to come up with any other way that would result in both type names to end up in the .rootmap file. I wouldn't hold this PR for an improvement, but we should follow up in #49458.
| <class name="reco::CaloRecHitHostCollection" ClassVersion="3"> | ||
| <version ClassVersion="3" checksum="1876594952"/> | ||
| </class> |
There was a problem hiding this comment.
I also removed the explicit versions and checksums for the PortableHostCollections that still had it. Their versioning should be done in a different way because PortableHostCollection<T> is a class template.
There was a problem hiding this comment.
I assume that we could extend the mechanism documented at core/dictgen/res/DictSelectionReader.h which allows the selection to be done via C++ code/template instantiation rather than xml or linkdef.
|
|
||
| <class name="lst::LSTInputHostCollection"/> | ||
| <class name="edm::Wrapper<lst::LSTInputHostCollection>" splitLevel="0"/> | ||
| <class name="edm::Wrapper<lst::LSTInputHostCollection>" splitLevel="0" persistent="false"/> |
There was a problem hiding this comment.
I also marked the LST collections as transient (by policy non-DataFormat packages should not define persistable data products).
|
enable gpu |
|
code-checks |
|
+code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-48824/47300
|
|
@cmsbuild, please test |
|
+1 Size: This PR adds an extra 16KB to repository Comparison SummarySummary:
AMD_W7900 Comparison SummarySummary:
NVIDIA_H100 Comparison SummarySummary:
NVIDIA_L40S Comparison SummarySummary:
NVIDIA_T4 Comparison SummarySummary:
|
|
How should we proceed with this PR? Given that the omission of the I see #49734 was opened to remove the |
4 participants
PR description:
Testing ROOT PR on an option to disable header parsing during
TClass::GetClass()call (root-project/root#18402) it was noticed the type alias gets listed in therootmapfile only if the alias is requested before the real type (see root-project/root#19705 for more details).In the mean time, having the type alias in the
rootmapfile is necessary to avoid header parsing for the execution of the read rules that use the type alias names, e.g.cmssw/DataFormats/Portable/interface/PortableHostCollectionReadRules.h
Line 84 in 2de4944
, and therefore it seemed worthwhile to change the dictionaries with
PortableHost{Collection,Object}.In the present state this PR demonstrates what the impact would be for the
PortableTestObjects. If deemed viable, the next steps would be to updateDataFormats/PortableREADME andscripts, and then update all the otherclasses_def.xmlfiles that declare these portable data products.Resolves cms-sw/framework-team#1542
PR validation:
The
testHeterogeneousCoreAlpakaTestWriteReadSerialSyncunit test succeeds with the build of cms-sw/root#222 (comment). The resultingrootmapcontains the::Product,::Layout, and::Implementationtype aliases that the read rules use.