DD Read Only View

[ianna]

• Move position of the "real" root node of the graph to compact view.
• It is initialized at construction time in EventSetup.
• Expanded view uses a read-only access to StoreT::instances

[cmsbuild]

A new Pull Request was created by @ianna for CMSSW_7_2_X.

DD Read Only View

It involves the following packages:

DetectorDescription/Core

@civanch, @Dr15Jones, @ianna, @mdhildreth, @cmsbuild, @nclopezo, @Degano, @ktf can you please review it and eventually sign? Thanks.
You can sign-off by replying to this message having '+1' in the first line of your reply.
You can reject by replying to this message having '-1' in the first line of your reply.

[Dr15Jones]

Looks like a good way to do it to me.

[ianna]

@Dr15Jones - please, review the code. I'm not sure why filtered view should always create an expanded view from the root node of a graph and not from a specific node. BTW, it is done ~30 times in a relval. I'll see if this can be improved.

[Dr15Jones]

+1

[Dr15Jones]

What about using a std::unique_ptr<DDPosData> and std::unique_ptr<DDCompactViewImpl> instead to automatically handle memory issues?

[ianna]

Thanks, @Dr15Jones I'll check it. The pointer is used in a container. I have to verify if all needed operations are possible - it is if they involve copying which the type does not support.

[cmsbuild]

This pull request is fully signed and it will be integrated in one of the next CMSSW_7_2_X IBs unless changes or unless it breaks tests.

[cmsbuild]

-1

Tested at: 74753d8
I ran the usual tests and I found errors in the following unit tests:

---> test const_dump had ERRORS
---> test DDErrorReport had ERRORS
---> test DDCompareCPV had ERRORS
---> test testNew had ERRORS
---> test reg_ddparser had ERRORS
---> test DetectorDescriptionParserTE had ERRORS
---> test reg_ddcore had ERRORS
---> test DetectorDescriptionRegressionTestDDErrorReport had ERRORS
---> test testmat had ERRORS
---> test testDDCompactView had ERRORS
---> test dumpDDCompactView had ERRORS
---> test dumpDDExpandedView had ERRORS

you can see the results of the tests here:
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-4476/237/summary.html

[cmsbuild]

This pull request is fully signed and it will be integrated in one of the next CMSSW_7_2_X IBs unless changes (but tests are reportedly failing).

[cmsbuild]

Pull request #4476 was updated. @civanch, @Dr15Jones, @ianna, @mdhildreth, @cmsbuild, @nclopezo, @Degano, @ktf can you please check and sign again.

[ianna]

@cmsbuild - this should fix the errors.

[cmsbuild]

-1

Tested at: e67508a
I ran the usual tests and I found errors in the following unit tests:

---> test DetectorDescriptionParserTE had ERRORS
---> test dumpDDExpandedView had ERRORS
---> test testDDCompactView had ERRORS

you can see the results of the tests here:
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-4476/239/summary.html

[cmsbuild]

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

[ianna]

@cmsbuild - oops, missed the default constructor. Fixed.

[cmsbuild]

Pull request #4476 was updated. @civanch, @Dr15Jones, @ianna, @mdhildreth, @cmsbuild, @nclopezo, @Degano, @ktf can you please check and sign again.

[cmsbuild]

+1
Tested at: eb71b99
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-4476/245/summary.html

[cmsbuild]

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

[ianna]

+1

[cmsbuild]

This pull request is fully signed and it will be integrated in one of the next CMSSW_7_2_X IBs unless changes (tests are also fine).