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

DD4hep: DD World Built from a DD4hep-based Geometry #26383

Closed
wants to merge 3 commits into from

Conversation

ianna
Copy link
Contributor

@ianna ianna commented Apr 8, 2019

PR description:

  • DDWorld to access G4 geometry
  • Converter to G4 geometry test

PR validation:

@mrodozov - this PR will not compile until the dd4hep-geant4 tool is setup correctly. When it does, it can be merged.

if this PR is a backport please specify the original PR:

no back port is needed

@cmsbuild
Copy link
Contributor

cmsbuild commented Apr 8, 2019

The code-checks are being triggered in jenkins.

@cmsbuild
Copy link
Contributor

cmsbuild commented Apr 8, 2019

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-26383/9133

  • This PR adds an extra 16KB to repository

@cmsbuild
Copy link
Contributor

cmsbuild commented Apr 8, 2019

A new Pull Request was created by @ianna (Ianna Osborne) for master.

It involves the following packages:

SimG4Core/DD4hepGeometry

The following packages do not have a category, yet:

SimG4Core/DD4hepGeometry
Please create a PR for https://github.com/cms-sw/cms-bot/blob/master/categories_map.py to assign category

@cmsbuild can you please review it and eventually sign? Thanks.
@makortel this is something you requested to watch as well.
@davidlange6, @slava77, @fabiocos you are the release manager for this.

cms-bot commands are listed here

@mrodozov
Copy link
Contributor

mrodozov commented Apr 8, 2019

@ianna I fixed the includes but now the code complains about "DDG4/Detector.h" not found in the DDG4 includes:
https://github.com/cms-sw/cmssw/pull/26383/files#diff-19bce7dd0d1ec0083d7be74adfb4e755R6
is it DD4hep/Detector.h ?

@cmsbuild
Copy link
Contributor

cmsbuild commented Apr 8, 2019

The code-checks are being triggered in jenkins.

@ianna
Copy link
Contributor Author

ianna commented Apr 8, 2019

Thanks @mrodozov ! Please, let me know when can I test it.

@cmsbuild
Copy link
Contributor

cmsbuild commented Apr 8, 2019

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-26383/9136

  • This PR adds an extra 16KB to repository

@cmsbuild
Copy link
Contributor

cmsbuild commented Apr 8, 2019

Pull request #26383 was updated. @cmsbuild can you please check and sign again.

@mrodozov
Copy link
Contributor

mrodozov commented Apr 9, 2019

please test

@cmsbuild
Copy link
Contributor

cmsbuild commented Apr 9, 2019

The tests are being triggered in jenkins.
https://cmssdt.cern.ch/jenkins/job/ib-any-integration/34066/console Started: 2019/04/09 09:55

@cmsbuild
Copy link
Contributor

cmsbuild commented Apr 9, 2019

-1

Tested at: c58a754

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

I found follow errors while testing this PR

Failed tests: Build ClangBuild

  • Build:

I found compilation error when building:

>> Package SimG4Core/DD4hepGeometry built
>> Entering Package SimG4Core/DD4hepGeometry
Entering library rule at SimG4Core/DD4hepGeometry
>> Compiling  /build/cmsbld/jenkins/workspace/ib-any-integration/CMSSW_10_6_X_2019-04-08-2300/src/SimG4Core/DD4hepGeometry/src/DD4hep_DDDWorld.cc 
/build/cmsbld/jenkins/workspace/ib-any-integration/CMSSW_10_6_X_2019-04-08-2300/src/SimG4Core/DD4hepGeometry/src/DD4hep_DDDWorld.cc: In constructor 'cms::DDDWorld::DDDWorld(const cms::DDDetector*)':
/build/cmsbld/jenkins/workspace/ib-any-integration/CMSSW_10_6_X_2019-04-08-2300/src/SimG4Core/DD4hepGeometry/src/DD4hep_DDDWorld.cc:19:38: error: binding reference of type 'dd4hep::Detector&' to 'const dd4hep::Detector' discards qualifiers
   dd4hep::sim::Geant4Converter g4Geo(detector);
                                      ^~~~~~~~
In file included from /build/cmsbld/jenkins/workspace/ib-any-integration/CMSSW_10_6_X_2019-04-08-2300/src/SimG4Core/DD4hepGeometry/src/DD4hep_DDDWorld.cc:4:0:
/cvmfs/cms-ib.cern.ch/nweek-02571/slc7_amd64_gcc700/external/dd4hep/v01-10x-dplpph/include/DDG4/Geant4Converter.h:58:7: note:   initializing argument 1 of 'dd4hep::sim::Geant4Converter::Geant4Converter(dd4hep::Detector&)'
       Geant4Converter(Detector& description);

  • Clang:

I found compilation error while trying to compile with clang. Command used:

USER_CUDA_FLAGS='--expt-relaxed-constexpr' USER_CXXFLAGS='-Wno-register -fsyntax-only' scram build -k -j 32 COMPILER='llvm compile'

gmake: *** [There are compilation/build errors. Please see the detail log above.] Error 1
+ eval scram build outputlog '&&' '(/build/cmsbld/jenkins/workspace/ib-any-integration/cms-bot/buildLogAnalyzer.py' --logDir /build/cmsbld/jenkins/workspace/ib-any-integration/CMSSW_10_6_X_2019-04-08-2300/tmp/slc7_amd64_gcc700/cache/log/src '||' 'true)'
++ scram build outputlog
Entering library rule at SimG4Core/DD4hepGeometry
>> Compiling  /build/cmsbld/jenkins/workspace/ib-any-integration/CMSSW_10_6_X_2019-04-08-2300/src/SimG4Core/DD4hepGeometry/src/DD4hep_DDDWorld.cc 
/build/cmsbld/jenkins/workspace/ib-any-integration/CMSSW_10_6_X_2019-04-08-2300/src/SimG4Core/DD4hepGeometry/src/DD4hep_DDDWorld.cc:19:32: error: no matching constructor for initialization of 'dd4hep::sim::Geant4Converter'
  dd4hep::sim::Geant4Converter g4Geo(detector);
                               ^     ~~~~~~~~
/cvmfs/cms-ib.cern.ch/nweek-02571/slc7_amd64_gcc700/external/dd4hep/v01-10x-dplpph/include/DDG4/Geant4Converter.h:32:11: note: candidate constructor (the implicit copy constructor) not viable: no known conversion from 'const dd4hep::Detector' to 'const dd4hep::sim::Geant4Converter' for 1st argument
    class Geant4Converter : public detail::GeoHandler, public Geant4Mapping {
          ^


@cmsbuild
Copy link
Contributor

cmsbuild commented Apr 9, 2019

Comparison not run due to Build errors (RelVals and Igprof tests were also skipped)

@cmsbuild
Copy link
Contributor

@cmsbuild
Copy link
Contributor

Comparison job queued.

@ianna
Copy link
Contributor Author

ianna commented Apr 10, 2019

assign simulation
assign geometry

@cmsbuild
Copy link
Contributor

New categories assigned: simulation

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

@ianna
Copy link
Contributor Author

ianna commented Apr 10, 2019

@fabiocos - this is a new package in Simulation with dependency on a dd4hep DDG4 static library.

@cmsbuild
Copy link
Contributor

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

Comparison Summary:

  • No significant changes to the logs found
  • Reco comparison results: 8 differences found in the comparisons
  • DQMHistoTests: Total files compared: 32
  • DQMHistoTests: Total histograms compared: 3140495
  • DQMHistoTests: Total failures: 1
  • DQMHistoTests: Total nulls: 0
  • DQMHistoTests: Total successes: 3140297
  • DQMHistoTests: Total skipped: 197
  • DQMHistoTests: Total Missing objects: 0
  • DQMHistoSizes: Histogram memory added: 0.0 KiB( 31 files compared)
  • Checked 133 log files, 14 edm output root files, 32 DQM output files

@civanch
Copy link
Contributor

civanch commented Apr 10, 2019

@ianna , I am a bit confused. If it is a test, why these are producers in plugin directory instead of test directory? What is the purpose of addition of watchers? What they intend to watch?

@civanch
Copy link
Contributor

civanch commented Apr 10, 2019

@ianna , if this package depends on geant4core, than geant4core cannot depend on DD4Hep. We have circular dependency. In the current situation, SimG4Core depends on DetectorDescription/Core. I would expect we will have the same thing. For today we have SimG4Core/Geometry/test which is intended for testing of geometry only. Why not reuse the same sub-package or creating a new one?

@ianna
Copy link
Contributor Author

ianna commented Apr 10, 2019

This package is intended to test sim dependency on dd4hep. The DDDWorld is a copy of our DDDWorld with an exception that it is in a cms namespace. So is the other class. I was wondering myself about implementations and took liberty to drop some assignments which looked useless to me.

If there’s a circular dependency, we have to address it.

My question is if the package affects the sim jobs in any way.

@civanch
Copy link
Contributor

civanch commented Apr 14, 2019

@ianna , current OscarMTProducer (which do real simulation) access would volume via old DD scheme of CMSSW. Your new code seems to provide an access to the world volume. For the test it is fine. It will be good to remove things, which are not connected with geometry in this new producer, this will ensure that we have no circular dependencies (after second look, I am not sure if we really have).

@ianna
Copy link
Contributor Author

ianna commented May 9, 2019

@civanch - there is no circular dependency. This package is needed to move further, e.g. to make a new OscarMTProducerFromDD4hep. However, it would help if it's integrated in an IB.

@smuzaffar smuzaffar modified the milestones: CMSSW_10_6_X, CMSSW_11_0_X May 14, 2019
@fabiocos
Copy link
Contributor

@ianna @civanch this PR looks now entirely embedded into #26740, which is fully approved, so I would say it may be closed

@ianna
Copy link
Contributor Author

ianna commented May 16, 2019

@fabiocos - yes, it's in #26740.

@ianna ianna closed this May 16, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants