vmc-project / vgm

Geometry conversion tool, actually providing conversion between Geant4 and ROOT TGeo geometry models.
GNU General Public License v3.0
5 stars 6 forks source link

Error while installing vgm with cern-root master branch #15

Closed Mehul0111 closed 7 months ago

Mehul0111 commented 7 months ago

I am trying to install vgm v5-2 with the cern-root master branch https://github.com/root-project/root.git and I am facing a problem in installation, as described below, from the class ... ROOTGM/source/Solids/Tessellated.cxx

[ 76%] Building CXX object packages/RootGM/CMakeFiles/RootGM.dir/source/solids/Tubs.cxx.o /home/metwi/CBMROOT_With_VecGeom/FairSoft_Build/Source/vgm/packages/RootGM/source/solids/TessellatedSolid.cxx: In member function ‘virtual VGM::ThreeVector RootGM::TessellatedSolid::Vertex(int, int) const’: /home/metwi/CBMROOT_With_VecGeom/FairSoft_Build/Source/vgm/packages/RootGM/source/solids/TessellatedSolid.cxx:184:26: error: ‘const class TGeoFacet’ has no member named ‘GetVertex’ 184 | vertex.push_back(facet.GetVertex(index).fVec[0] RootGM::Units::Length()); | ^~~~~ /home/metwi/CBMROOT_With_VecGeom/FairSoft_Build/Source/vgm/packages/RootGM/source/solids/TessellatedSolid.cxx:185:26: error: ‘const class TGeoFacet’ has no member named ‘GetVertex’ 185 | vertex.push_back(facet.GetVertex(index).fVec[1] RootGM::Units::Length()); | ^~~~~ /home/metwi/CBMROOT_With_VecGeom/FairSoft_Build/Source/vgm/packages/RootGM/source/solids/TessellatedSolid.cxx:186:26: error: ‘const class TGeoFacet’ has no member named ‘GetVertex’ 186 | vertex.push_back(facet.GetVertex(index).fVec[2] RootGM::Units::Length()); | ^~~~~ gmake[5]: [packages/RootGM/CMakeFiles/RootGM.dir/build.make:496: packages/RootGM/CMakeFiles/RootGM.dir/source/solids/TessellatedSolid.cxx.o] Error 1 gmake[5]: Waiting for unfinished jobs.... gmake[4]: [CMakeFiles/Makefile2:396: packages/RootGM/CMakeFiles/RootGM.dir/all] Error 2 gmake[4]: Waiting for unfinished jobs.... [ 77%] Linking CXX shared library libGeant4GM.so [ 77%] Built target Geant4GM gmake[3]: [Makefile:136: all] Error 2**

I also have attached the build.log file.

vgm-build.log

in the legacy file you can see the different tag of clhep, geant4, root, and VecGeom.

legacy.txt

@ihrivnac could you please help to solve this issue?

olifre commented 7 months ago

Looks like ROOT master broke compatibility with quite some geometry libraries as of https://github.com/root-project/root/commit/aeffc7467d0034c7504229ec9cf52a045316f747 Other affected libraries, e.g. DD4hep, are also mentioned there.

However, it seems this is not part of any ROOT release yet, so you will only run into this problem when using an unreleased version of ROOT built from master.

Mehul0111 commented 7 months ago

@olifre together with one of my Friend/Colleague (Om) while working on implimentation of geometry from GDML file to ROOT. We have seen the problem of percistency for the Tessellated solids when saved in .root file even after using convertor before closing the geometry.

//Convert to VecGeom tessellated solid auto converter = TVirtualGeoConverter::Instance(gGeoManager); if (!converter) { printf("Raytracing a tessellated shape without VecGeom support will just draw a box\n"); } else { converter->ConvertGeometry(); }

Because of the persistency problem, I think, it was just wraping the Tessellated solids volume information in the root file and still was behaving like a box event after Convertor was used. It was not recognizing by the navigator from where the actual volume boundries starts and end when emplyoing with random rays/points for a test. Therefore the issue was created in ROOT github by OM https://github.com/root-project/root/issues/14283

The problem was then solved by A. Gheata and necessary changes updated into the ROOT TGeoTessellated and TVGShape class (.cxx and .h files) and other places where needed.

What could be the step taken here now to avoid the problem appearing. Will those changes made in ROOT to solve the presistency problem for Tessellated solids be also adapted in next release of VGM or directly to v5-2 VGM?

ihrivnac commented 7 months ago

Hello, Thank you, @Mehul0111 , for reporting this problem. I will take a look if we can use another access method instead of that which is now removed, and if not, I will report a problem in ROOT. Best regards,

agheata commented 7 months ago

Indeed, as @Mehul0111 mentioned, the backward incompatibility was necessary to fix the broken persistency design of the tessellations. So facets do not store direct vertex information, but only vertex indices now. The global array of vertices is available in the TGeoTessellated object, so the way to access a face vertex changes from: facet.GetVertex(i) to tessellated.GetVertex(facet[i]). Sorry for the trouble, I did not find another easy way to sort this out w/o breaking the interface.

olifre commented 7 months ago

I think the good news might be that tesselated.GetVertex(index) was already there before the change, so it might be possible for libraries such as VGM to use this API already now and thus be compatible with the currently released ROOT version and the upcoming ROOT version.

If that is true, it would avoid the pain of requiring a new release of all libraries which would then not work with existing ROOT versions, but only upcoming ones (which is quite painful when packaging for Linux distros). That would also answer the question by @Mehul0111 .

ihrivnac commented 7 months ago

I think the good news might be that tesselated.GetVertex(index) was already there before the change, so it might be possible for libraries such as VGM to use this API already now and thus be compatible with the currently released ROOT version and the upcoming ROOT version.

However, in the tagged version (6.30/04) , the facet does not provide the [] operator, so the solution is not backward compatible. I will put the test for the ROOT version, and when a ROOT tag with this change is available, a new tag in VGM.

ihrivnac commented 7 months ago

@Mehul0111 , the problem should be now fixed in master. Please, let me know if it works ok for you.

olifre commented 7 months ago

However, in the tagged version (6.30/04) , the facet does not provide the [] operator, so the solution is not backward compatible. I will put the test for the ROOT version, and when a ROOT tag with this change is available, a new tag in VGM.

Good catch, many thanks! I'll keep that in mind during packaging for Gentoo (I need to replicate these dependencies there, i.e. VGM 5.2 and lower will get a dependency on ROOT versions below the to-be-released tag, and the new VGM tag will then work with current and future ROOT versions). Thanks!

Mehul0111 commented 7 months ago

@ihrivnac and @olifre Thank you so much for the changes and your help.

I have tried to install the ROOT master branch and vgm with the changes you made. It is successfully installed.

Also, tested my geometry, looks working fine.

Screenshot from 2024-04-01 18-35-32

Screenshot from 2024-04-01 18-33-14

ihrivnac commented 7 months ago

Thank you, @Mehul0111 , for testing !