Allow the creation of a(n almost) full geometry w/o a prior VMC instance by aphecetche · Pull Request #1113 · AliceO2Group/AliceO2

aphecetche · 2018-05-24T13:19:19Z

Decoupling TGeo material creation from VMC (i.e. can now build TGeo material without having to instantiate first a TGeant3TGeo or TGeant4 object)

This is done by inverting the current responsabilities. TGeo is now the master in charge of
creating the geometry, and VMC is just the client taking the geometry
from TGeo (and possibly tweaking media properties which cannot be
handled by TGeo itself).

The net advantage is to get a lighter dependency tree and is useful e.g. to build a geometry within a unit test.

…nstance. This is done by inverting the current responsabilities. TGeo is now the master in charge of creating the geometry, and VMC is just the client taking the geometry from TGeo (and possibly tweaking media properties which cannot be handled by TGeo itself).

alibuild · 2018-05-24T13:23:11Z

Error while checking build/O2/o2-dev-fairroot for a4d7427:

sw/BUILD/O2-latest/log
  CMake did not find one.

  Could not find a package configuration file provided by "DDS" with any of
  the following names:

    DDSConfig.cmake
    dds-config.cmake

  Add the installation prefix of "DDS" to CMAKE_PREFIX_PATH or set "DDS_DIR"
  to a directory containing one of the above files.  If "DDS" provides a
  separate development package or SDK, be sure it has been installed.
Call Stack (most recent call first):
  CMakeLists.txt:65 (include)


-- Boost version: 1.66.0
-- Found the following Boost libraries:
--   thread
--   system
--   timer
--   program_options
--   random
--   filesystem
--   chrono
--   exception
--   regex
--   serialization
--   log
--   log_setup
--   unit_test_framework
--   date_time
--   signals
--   atomic
-- Setting FairRoot environment…
-- FairRoot ... - found /mnt/mesos/sandbox/sandbox/sw/slc7_x86-64/FairRoot/dev-1
-- FairRoot Library directory  :     /mnt/mesos/sandbox/sandbox/sw/slc7_x86-64/FairRoot/dev-1/lib
-- FairRoot Include path…      :     /mnt/mesos/sandbox/sandbox/sw/slc7_x86-64/FairRoot/dev-1/include
-- FairRoot Cmake Modules      :     /mnt/mesos/sandbox/sandbox/sw/slc7_x86-64/FairRoot/dev-1/share/fairbase/cmake
-- FairRoot ... - found /mnt/mesos/sandbox/sandbox/sw/slc7_x86-64/FairRoot/dev-1
-- Looking for FairMQ functionality in FairRoot ...
CMake Error at cmake/modules/FindFairMQ.cmake:46 (message):
  FairRoot is not built with FairMQ support
Call Stack (most recent call first):
  cmake/O2Dependencies.cmake:59 (find_package)
  CMakeLists.txt:65 (include)


-- Configuring incomplete, errors occurred!
See also "/mnt/mesos/sandbox/sandbox/sw/BUILD/f2a85d8d271a0d60da22b4f3a34a735befb8c5c1/O2/CMakeFiles/CMakeOutput.log".

Full log here.

sawenzel · 2018-05-24T14:47:17Z

Nice PR. I will check if the simulation still runs exactly as before by cross-checking the number of steps done in each module (using the StepLogger). If all goes fine, this will go in.

sawenzel · 2018-05-24T15:31:07Z

I am afraid that the first QA check did not pass. I ran the following command on dev and here:

DYLD_INSERT_LIBRARIES=libMCStepLogger.dylib o2sim -m FIT EMCAL TRD TPC ITS -n 1 -g pythia8

This is using a random selection of modules. I see a totally different number of steps done and much less secondaries for this new PR.

sawenzel

We have to block this until we can show that it doesn't affect simulation.

aphecetche · 2018-05-24T15:35:16Z

ok. will get back to the drawing board then ...

sawenzel · 2018-05-24T15:45:18Z

Can we maybe close this PR and reopen later? The CI servers will be happy.

shahor02

Hi @aphecetche

At least for the FIT this hits the problem I've mentioned before in the alice-talk: the optical properties are set in the ConstructGeometry directly on VMC media, which means that the VMC must already contain these materials. Your trick with run->SetImportTGeoToVMC(true); in o2sim will not help here, since the FairMCApplication calls

  if (fRun->IsImportTGeoToVMC() ) {
    TVirtualMC::GetMC()->SetRootGeometry();         // notify VMC about Root geometry
    LOG(info) << "TGeometry will be imported to VMC" << "\n";
  }

after the loop over the ConstructGeometry for every module.

aphecetche · 2018-05-24T17:34:00Z

Hi @shahor02

Indeed that must be the issue, and there's a massive difference because of the TPC SetSpecialPhysicsCuts call... (e.g. just using ITS or commenting out the SetSpecialPhysicsCuts calls, I get the same hit counts)

Have to think a bit ...

shahor02 · 2018-05-24T17:43:24Z

@aphecetche : I think, also detectors which ConstructGeometry directly via TVirtualMC::GetMC()->Gsvolu (Calos, FIT,PHOS,TOF,TRD) should get problem, since they
use in ConstructGeometry media names which should be known to VMC.

Cheers,
Ruben

sawenzel · 2018-05-24T17:45:54Z

Well for the moment you could just use in the MaterialManager (like in Detector): if(vmc) { } else { // use ROOT }

This should leave the simulation intact and allowing you to test geometry standalone.

The other way around does not yield the same simulation results, and this is not understand for the moment. Until it is, VMC is kept as the driver, while TGeo can be used standalone to e.g. test to geometry.

alibuild · 2018-05-28T08:33:31Z

Error while checking build/O2/o2-dev-fairroot for 543a6b6:

sw/BUILD/O2-latest/log
  CMake did not find one.

  Could not find a package configuration file provided by "DDS" with any of
  the following names:

    DDSConfig.cmake
    dds-config.cmake

  Add the installation prefix of "DDS" to CMAKE_PREFIX_PATH or set "DDS_DIR"
  to a directory containing one of the above files.  If "DDS" provides a
  separate development package or SDK, be sure it has been installed.
Call Stack (most recent call first):
  CMakeLists.txt:65 (include)


-- Boost version: 1.66.0
-- Found the following Boost libraries:
--   thread
--   system
--   timer
--   program_options
--   random
--   filesystem
--   chrono
--   exception
--   regex
--   serialization
--   log
--   log_setup
--   unit_test_framework
--   date_time
--   signals
--   atomic
-- Setting FairRoot environment…
-- FairRoot ... - found /mnt/mesos/sandbox/sandbox/sw/slc7_x86-64/FairRoot/dev-1
-- FairRoot Library directory  :     /mnt/mesos/sandbox/sandbox/sw/slc7_x86-64/FairRoot/dev-1/lib
-- FairRoot Include path…      :     /mnt/mesos/sandbox/sandbox/sw/slc7_x86-64/FairRoot/dev-1/include
-- FairRoot Cmake Modules      :     /mnt/mesos/sandbox/sandbox/sw/slc7_x86-64/FairRoot/dev-1/share/fairbase/cmake
-- FairRoot ... - found /mnt/mesos/sandbox/sandbox/sw/slc7_x86-64/FairRoot/dev-1
-- Looking for FairMQ functionality in FairRoot ...
CMake Error at cmake/modules/FindFairMQ.cmake:46 (message):
  FairRoot is not built with FairMQ support
Call Stack (most recent call first):
  cmake/O2Dependencies.cmake:59 (find_package)
  CMakeLists.txt:65 (include)


-- Configuring incomplete, errors occurred!
See also "/mnt/mesos/sandbox/sandbox/sw/BUILD/da83bbb27b96e6f50d85d24a94a256ceb97a5533/O2/CMakeFiles/CMakeOutput.log".

Full log here.

aphecetche · 2018-05-28T08:47:01Z

@sawenzel

indeed, after a bunch of inconclusive tests (meaning I still don't understand why I don't get the expected same results with TGeo driving the geometry creation), I added a couple of if in MaterialManager for the moment, to get the thing moving forward.

Created a JIRA ticket for future work.

sawenzel · 2018-05-28T14:46:54Z

 #include "DetectorsBase/MaterialManager.h"
-#include <TVirtualMC.h>  // for TVirtualMC, gMC
-#include "TString.h"     // for TString
+#include "TVirtualMC.h"


why these changes? They are not necessary for this PR, I guess.

hum, no, indeed. In the original commit that include was no longer needed, then I put it back in the second commit but as a quotes include instead of angled include, out of habit I guess (as I've never quite understood why is Root considered a system include ;-) )

sawenzel · 2018-05-29T10:07:54Z

I squashed the 2 commits since it is not interesting to have the intermediate (breaking) state.

…nce (AliceO2Group#1113) * Allow the creation of a(n almost) full geometry without a prior VMC instance. Whenever a VMC instance is not present, allow to use direct TGeo constructs for material/medium construction. This allows to perform standalone tests without having to instantiate simulation.

sawenzel requested review from sawenzel and shahor02 May 24, 2018 15:31

sawenzel requested changes May 24, 2018

View reviewed changes

aphecetche closed this May 24, 2018

shahor02 reviewed May 24, 2018

View reviewed changes

Get back to VMC driving the geometry creation by default.

543a6b6

The other way around does not yield the same simulation results, and this is not understand for the moment. Until it is, VMC is kept as the driver, while TGeo can be used standalone to e.g. test to geometry.

aphecetche reopened this May 28, 2018

sawenzel reviewed May 28, 2018

View reviewed changes

sawenzel approved these changes May 28, 2018

View reviewed changes

sawenzel merged commit 18a680c into AliceO2Group:dev May 29, 2018

mikesas pushed a commit to mikesas/AliceO2 that referenced this pull request Dec 13, 2022

change type (AliceO2Group#1113)

b25fd48

Conversation

aphecetche commented May 24, 2018

Uh oh!

alibuild commented May 24, 2018

Uh oh!

sawenzel commented May 24, 2018

Uh oh!

sawenzel commented May 24, 2018

Uh oh!

sawenzel left a comment

Choose a reason for hiding this comment

Uh oh!

aphecetche commented May 24, 2018

Uh oh!

sawenzel commented May 24, 2018

Uh oh!

shahor02 left a comment

Choose a reason for hiding this comment

Uh oh!

aphecetche commented May 24, 2018

Uh oh!

shahor02 commented May 24, 2018

Uh oh!

sawenzel commented May 24, 2018

Uh oh!

alibuild commented May 28, 2018

Uh oh!

aphecetche commented May 28, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

sawenzel May 28, 2018

Choose a reason for hiding this comment

Uh oh!

aphecetche May 28, 2018

Choose a reason for hiding this comment

Uh oh!

sawenzel commented May 29, 2018

Uh oh!

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

4 participants

aphecetche commented May 28, 2018 •

edited

Loading