Problem 1220 - Pitfall in G4Material
Summary: Pitfall in G4Material
Status: RESOLVED FIXED
Alias: None
Product: Geant4
Classification: Unclassified
Component: materials (show other problems)
Version: 9.4
Hardware: All All
: P3 normal
Assignee: Vladimir.Ivantchenko
URL:
Depends on:
Blocks:
 
Reported: 2011-06-06 22:30 CEST by Wm. Heintzelman
Modified: 2011-06-09 11:54 CEST (History)
1 user (show)

See Also:


Attachments
Contains suggested changes to classes G4Material and G4NistMaterialBuilder (22.55 KB, application/x-gzip)
2011-06-06 22:30 CEST, Wm. Heintzelman
Details

Note You need to log in before you can comment on or make changes to this problem.
Description Wm. Heintzelman 2011-06-06 22:30:38 CEST
Created attachment 120 [details]
Contains suggested changes to classes G4Material and G4NistMaterialBuilder

The class G4Material contains a data member "fIndexInTable", which is the index of the material in theMaterialTable.  There is also a public function, G4Materials::GetIndex() that allows access to fIndexInTable from outside the class.  However, there is no provision in the class destructor ~G4Material to adjust the values of fIndexInTable for other class members if one member is deleted, which a Geant user may want to do.  (For example, one might write a general material building routine that doesn't require the builds to be done in strict order with all constituents necessarily completed before a material build is attempted.)  After such a deletion, the values of fIndexInTable will be incorrect for all materials built after the one that was deleted.

I think it would be best to remove fIndexInTable from the class definition and recode where necessary to avoid usage of GetIndex().  However, the comments in the class header dated 14-09-01 and 26-02-02 indicate that this had been done in the past, and then reversed.  If fIndexInTable is retained, provision should be made for destruction of a class instance without introducing an error.  The versions of G4Materials.hh and G4Materials.cc in the attached tar file have a new class function "Delete()" that adjusts the other fIndexInTable values before deleting the member indicated in the function call.

The class G4NistMaterialBuilder also maintains a private table of material index values, matIndex, obtained by calling G4Material::GetIndex().  If matIndex is retained, it also would need to be updated when a material is deleted.  However, that class can easily be recoded to avoid the use of G4Material::GetIndex().  Versions of G4NistMaterialBuilder.cc and G4NistMaterialBuilder.hh that have been recoded accordingly are also in the tar file.

A grep of the Geant4 source files indicates that there are many calls to functions named "GetIndex".  The code should be reviewed to ensure that none of the usages of G4Material::GetIndex is based on the implicit assumption that a material is never destroyed after once having been created.
Comment 1 Vladimir.Ivantchenko 2011-06-08 15:21:17 CEST
There are problems with the proposal for the modifications of G4Material and G4NistMaterialBuilder classes: 
1) proposed modification on top of 9.4p01 cannot be implemented because code is changed significantly - variable base material approach is added and the code was cleaned up
2) it is not allowed to delete G4Material by user code

The last is our policy there is no sense to delete this tiny class - no advantages for Physics Tables size (number of materials is not connected with table size), the memory will likely not be reused. Unfortunately, we do not protect G4Material for unwanted deletion but likely should do that for the December release.

Final comment: the use-case is unclear - likely all problem can be resolved by modification of user code only (for example, by changing of order of material build and by removing of "delete" of intermediate materials). For that we need to see the code. In this sense it is not a complete bug report.

VI
.
Comment 2 Wm. Heintzelman 2011-06-08 16:24:02 CEST
Yes, it is correct that the problem can be avoided by modification of the user code.  The point is that there is nothing (at least nothing that I can find) in the G4Material header file or in the User's Guide to warn the user that deletion of a G4Material instance is not allowed, and there is nothing to prevent him from doing so.

An adequate solution would be to protect G4Material from unwanted deletion and in addition to document both in G4Material.hh and in the User's Guide that deletion of a material by user code is not allowed.
Comment 3 Vladimir.Ivantchenko 2011-06-08 16:36:21 CEST
Hello,

The improved comment will be included into 9.5beta version. Will do today. A software protection is also possible but should be discussed before implementation.

VI
Comment 4 Vladimir.Ivantchenko 2011-06-09 11:54:13 CEST
Comments to G4Material.hh and G4Elemnt.hh have been reviewed and updated.

VI