This is the same class where the bug #409 was reported (not all implementations of MakeImprint() method were updating the imprints counter). This time I found the second bug: the implementation of the counter of instances is incorrect. Here is what's happening. The class G4AssemblyVolume has a private static member "static unsigned int fsInstanceCounter." Each time a new instance is created, the variable fsInstanceCounter is incremented. Thus, all instances of the class have one and the same value of fsInstanceCounter (since it is static). Specifically, when the "G4AssemblyVolume av1" is declared, it has the fsInstanceCounter equal to 1, but the moment "G4AssemblyVolume av2" is created, BOTH av1 and av2 have the SAME counter value equal to 2. As a result, when two (or more) assembly volumes are created and then the method MakeImprint() is used to create imprints, the name of the physical volumes generated so all start with the same "av_2." The correct way would be for the imprints of the first assembly volume to start with "av_1" and for the imprints of the second volume to start with "av_2." Here is the fastest (but not the most correct way) to fix the problem. Introduce one more private variable to the class: put the following line into the header of the class, into the private section: unsigned int fsInstanceIDofThisObject; This variable is not static, it will hold the ID (i.e. the counter value) for each instance of the class. The old fsInstanceCounter variable will be keeping track of which ID number was used last. One more correction in the header is needed: a function that would report the ID of an instance. Hence, add the following line to the header's public section: unsigned int GetInstanceID() const; All other corrections have to be made in the source code, i.e., G4AssemblyVolume.cc file. Here are all those corrections listed: 1. Add the line fsInstanceIDofThisObject = fsInstanceCounter; as the last line in the constructor. 2. In both implementations of MakeImprint() method, change the operator pvName << "av_" << GetInstanceCount() << "_impr_" << GetImprintsCount() << "_" << fTriplets[i].GetVolume()->GetName().c_str() << "_pv_" << i << G4std::ends; to read as pvName << "av_" << GetInstanceID() << "_impr_" << GetImprintsCount() << "_" << fTriplets[i].GetVolume()->GetName().c_str() << "_pv_" << i << G4std::ends; I.e., the private to each instance ID value should be used in forming the name of the physical volume, and not the current number of assembly volumes in existence (which is what you've get if GetInstanceCount() is used). 3. Add the implementation of the method GetInstanceID() to the source code (the declaration of it was already added to the header): unsigned int G4AssemblyVolume::GetInstanceID() const { return fsInstanceIDofThisObject; } 4. It probably makes sense to add the line fsInstanceIDofThisObject = fsInstanceCounter; as the last line of SetInstanceCount() method. However, I personally do not agree with the fact that fsInstanceCounter is forces to play a double role here - not only does it have to count the number of instances, but it also has to beware of the initial value to start with. The SetInstanceCount(), and the whole idea of using the same variable for two roles, creates potential for a bug. I implemented all the corrections described above in the version of code on my hard drive, and tested it. I recommend similar corrections to be performed in the official version of Geant4.
Thanks for pointing that out. Fix on CVS HEAD. Cheers Radovan Apply cvs diff bellow if you don't have direct access to CVS: Index: include/G4AssemblyVolume.hh =================================================================== RCS file: /afs/cern.ch/sw/geant4/cvs/geant4/source/geometry/management/include/G4Ass emblyVolume.hh,v retrieving revision 1.7 retrieving revision 1.8 diff -r1.7 -r1.8 24c24 < // $Id: G4AssemblyVolume.hh,v 1.7 2002/06/22 00:39:23 radoone Exp $ --- > // $Id: G4AssemblyVolume.hh,v 1.8 2002/09/10 16:59:44 radoone Exp $ 175a176,178 > // Return the number of existing instance of G4AssemblyVolume class > unsigned int GetAssemblyID() const; > // Return instance number of this concrete object 179a183,184 > void SetAssemblyID( unsigned int value ); > 185a191,193 > // Class instance counter > unsigned int fAssemblyID; > // Assembly object ID derived from instance counter at construction time Index: include/G4AssemblyVolume.icc =================================================================== RCS file: /afs/cern.ch/sw/geant4/cvs/geant4/source/geometry/management/include/G4Ass emblyVolume.icc,v retrieving revision 1.3 retrieving revision 1.4 diff -r1.3 -r1.4 24c24 < // $Id: G4AssemblyVolume.icc,v 1.3 2001/07/11 09:59:16 gunter Exp $ --- > // $Id: G4AssemblyVolume.icc,v 1.4 2002/09/10 16:59:46 radoone Exp $ 55a56,68 > > inline > unsigned int G4AssemblyVolume::GetAssemblyID() const > { > return fAssemblyID; > } > > inline > void G4AssemblyVolume::SetAssemblyID( unsigned int value ) > { > fAssemblyID = value; > } > Index: src/G4AssemblyVolume.cc =================================================================== RCS file: /afs/cern.ch/sw/geant4/cvs/geant4/source/geometry/management/src/G4Assembl yVolume.cc,v retrieving revision 1.10 retrieving revision 1.13 diff -r1.10 -r1.13 24c24 < // $Id: G4AssemblyVolume.cc,v 1.10 2002/06/24 07:15:27 gcosmo Exp $ --- > // $Id: G4AssemblyVolume.cc,v 1.13 2002/09/10 17:07:15 radoone Exp $ 44a45 > : fAssemblyID( 0 ) 46a48 > SetAssemblyID( GetInstanceCount() ); 195c197 < << GetInstanceCount() --- > << GetAssemblyID() 255a258,259 > ImprintsCountPlus(); > 269c273 < << GetInstanceCount() --- > << GetAssemblyID()