In 10.7 there has been a API change that directly affects users (one example collaboration is opticks) The files affected are ./source/geometry/volumes/include/G4LogicalBorderSurface.hh ./source/geometry/volumes/src/G4LogicalBorderSurface.cc ./source/persistency/gdml/src/G4GDMLWriteStructure.cc now Geant4 uses std::map instead of std::vector to define G4LogicalBorderSurfaceTable using G4LogicalBorderSurfaceTable = std::map<std::pair<const G4VPhysicalVolume*, const G4VPhysicalVolume*>, G4LogicalBorderSurface*>; instead of previously (before 10.7): using G4LogicalBorderSurfaceTable = std::vector<G4LogicalBorderSurface*>; So any code that assumes a vector as return will fail. I understand the change was motivated by making Surface finding faster see https://github.com/Geant4/geant4/pull/6 but this shouldn't be done by changing the user interface. I think there are three possible solutions that should be discussed: 1.) revert all the three files affected back to what they were before 10.7 2.) revert ./source/geometry/volumes/include/G4LogicalBorderSurface.hh ./source/geometry/volumes/src/G4LogicalBorderSurface.cc back to what they were before 10.7 but modify G4GDMLWriteStructure.cc to use a map internally to improve the efficiency of surface finding. 3.) extend the interface to provide both a map or a vector. Any method to retrieve the vectors would stay the same but there would be additional methods that would return a map (e.g. to be used in: G4GDMLWriteStructure) using G4LogicalBorderSurfaceMap = std::map<std::pair<const G4VPhysicalVolume*, const G4VPhysicalVolume*>, G4LogicalBorderSurface*>; using G4LogicalBorderSurfaceTable = std::vector<G4LogicalBorderSurface*>; please comment.
The introduced change comes with a *NEW* Geant4 release. Minor user code migrations can be requested when moving to a new minor Geant4 release. Solutions 2] and 3] don't make sense; the change has not been done for GDML but to speed up look-up of surfaces in memory and does not make sense to double the internal memory structure (with consequent increase of memory footprint). It is not clear to my *why* codes like Opticks cannot update to the new signature in the prospect of migrating to release 10.7 of Geant4, can you please explain? The only possible alternative I can see is to revert back the change in 10.7.p01, for the only motivation that documentation of the current change was missed in the release notes - and this would be unfortunate; the new development would in any case remain in the development releases and future new releases!
Of course opticks can be changed but that's not the point. This really represents a "hidden" change to the API where the signature doesn't change but the type of the classes that are passed. Also if that change is advantageous why wasn't it done for e.g. G4LogicalSkinSurface.hh:using G4LogicalSkinSurfaceTable = std::vector<G4LogicalSkinSurface*>; just to be consistent.
It is indeed an API change and 'm sorry for the mistake to have missed mentioning it in the release notes, so it would have not been "hidden"... it was certainly not the intention. Skin and Border surfaces are not the same thing; for skin-surfaces a simple vector structure can efficiently do the job; for border-surfaces, applying to volumes couples, a map is far best suited.
std::map< std::pair<const G4VPhysicalVolume*, const G4VPhysicalVolume*>, G4LogicalBorderSurface*> The std::pair of G4VPhysicalVolume* pointers key means that the std::map iteration order obtained by comparing the keys in not controlled. The ordering may change from invokation to invokation and might be different on different systems. As Opticks serializes all geometry objects into arrays ready for upload as GPU buffers/textures it is necessary for all geometry objects including bordersurface to have a well defined ordering in their collections as indices are used to refer to them. My workaround for coping with the std::map in recent Opticks commits is to impose a more meaningful and consistent ordering by sorting the initial std::vector I obtain from the std::map based on the stripped name of G4LogicalBorderSurface. This is done in X4LogicalBorderSurfaceTable::PrepareVector * https://bitbucket.org/simoncblyth/opticks/src/master/extg4/X4LogicalBorderSurfaceTable.cc Although the name sorting workaround seems fine for 1070 it would be preferable if it were possible for the well defined creation order pre-1070 to be regained post-1070 by recording a creation order index in the G4LogicalBorderSurface, for example by adding the below Index line in the constructor and a GetIndex accessor. 44 G4LogicalBorderSurface:: 45 G4LogicalBorderSurface(const G4String& name, 46 G4VPhysicalVolume* vol1, 47 G4VPhysicalVolume* vol2, 48 G4SurfaceProperty* surfaceProperty) 49 : G4LogicalSurface(name, surfaceProperty), 50 Volume1(vol1), Volume2(vol2), Index( theBorderSurfaceTable ? theBorderSurfaceTable->size() : 0 ) // Assign creation order index to the border surface 51 { 52 if (theBorderSurfaceTable == nullptr) 53 { 54 theBorderSurfaceTable = new G4LogicalBorderSurfaceTable; 55 } 56 57 // Store in the table of Surfaces 58 // 59 theBorderSurfaceTable->insert(std::make_pair(std::make_pair(vol1,vol2),this)); 60 } 61 Having a creation order index is also generally useful whilst debugging.
Thanks Simon for the suggestion. I agree adding an index can help and can be useful. This is now added for testing to the development branch and will be included in the next patch release as well.
