Problem 1283

Reassigning to track category.

Unfortunately, problem 1273 did not actually find the root of the problem, nor did my initial description here. A half hour with a debugger showed me where the problem actually lies (followed by some code comparisons between Geant4 versions).

The root of the problem is in the implementations of UpdateStepForPostStep(), and probably also UpdateStepForAlongStep() and UpdateStepForAtRest(). This problem was present in Geant4.9.4.p03. It was not in geant4.9.3.p02 but that had a different bug that was reported in problem 1243 -- the weight was updated in pPostStepPoint rather than in the track itself. So I believe that this was introduced in the fix to problem 1243.

In UpdateStepForPostStep(), the code to update track weights is inside an if(!fSetSecondaryWeightByProcess), which is right, but that if is inside an if(isParentWeightProposed), and that is wrong. If fSetSecondaryWeightByProcess is false, then the weights of the secondaries should be set to the weight of the parent, regardless of whether this process changed the parent's weight. 

Processes that generate secondaries and don't kill the parent should call SetSecondaryWeightByProcess(true) and handle the weights themselves (because in general it is not obvious what to do, and whatever choice is made in G4VParticleChange will be wrong for some process -- presumably this is why this function is available).

I was surprised that the code to update a secondary's weight was not in G4VParticleChange::AddSecondary() -- that seems like the logical place for it, and it is as close as possible to the point in the code where the process would set the weight if the process had called SetSecondaryWeightByProcess(true). The old method of updating the weight in pPostStepPoint has a high likelihood of over-writing the weight that the process set (and it cannot be correct for processes that generate secondaries but don't kill the parent).

I note that not all UpdateStepForPostStep(), UpdateStepForAlongStep(), and UpdateStepForAtRest() have the code to update the secondaries' weights. In the current approach, EVERY ONE of these functions must update the secondaries' weights if fSetSecondaryWeightByProcess is false, so there are a lot of bugs that have not yet been noticed.

I suggest removing this code from all the UpdateStepFor*() functions and putting it into G4VParticleChange::AddSecondary(). While there are several derived classes that override the base-class implementation, they all call G4VParticleChange::AddSecondary() to actually add the secondary track. So this would collapse 24 copies of the code to a single implementation (there are 8 classes derived from G4VParticleChange that implement the three UpdateStepFor* functions). It would also put the code where I believe it belongs (though your developers probably have a better handle on such things than I).

Just a quick note: Geant4.9.5.p01 did not fix this (no surprise, as it is still ASSIGNED). So I have to re-do all my edits, and there are a lot of them :-(.

Here are the files of geant4.9.5.p01 I modified related to this bug. In all cases the change is marked //TJR:
geant4.9.5.p01/source/track/src/G4ParticleChange.cc
geant4.9.5.p01/source/track/src/G4ParticleChangeForDecay.cc
geant4.9.5.p01/source/track/src/G4ParticleChangeForGamma.cc
geant4.9.5.p01/source/track/src/G4ParticleChangeForLoss.cc
geant4.9.5.p01/source/track/src/G4VParticleChange.cc

As mentioned in my second comment, there are other G4ParticleChangeFor... files which do not ever set the secondaries' weight. These are bugs waiting to be noticed. My modifications below fix them, too, by moving the setting of a secondary's weight from a loop over secondaries in G4ParticleChangeFor... to G4VParticleChange::AddSecondary(), where I believe it belongs.

The following code has been pasted multiple times into each file named above, and must be removed. Every instance is inside an "if (isParentWeightProposed) { }", which is the problem, as the secondary weight must be set even if this process did not modify the parent's weight.

 //TJR    if (!fSetSecondaryWeightByProcess) {
 //TJR      // Set weight of secondary tracks
 //TJR      for (G4int index= 0; index<theNumberOfSecondaries; index++){
 //TJR   if ( (*theListOfSecondaries)[index] ) {
 //TJR     ((*theListOfSecondaries)[index])->SetWeight(theParentWeight); ;
 //TJR   }
 //TJR      }
 //TJR    }


In file geant4.9.5.p01/source/track/src/G4VParticleChange.cc there is another change, which sets the secondary weight for every secondary (two new lines marked //TJR); note that all derived-class AddSecondary() functions call this one:

void G4VParticleChange::AddSecondary(G4Track *aTrack)
{
  if (debugFlag) CheckSecondary(*aTrack);

  // add a secondary after size check
  if (theSizeOftheListOfSecondaries > theNumberOfSecondaries) {
    if (!fSetSecondaryWeightByProcess)          //TJR
          aTrack->SetWeight(theParentWeight);   //TJR
    theListOfSecondaries->SetElement(theNumberOfSecondaries, aTrack);
    theNumberOfSecondaries++;
  } else {
    delete aTrack;

#ifdef G4VERBOSE
    if (verboseLevel>0) {
      G4cout << "G4VParticleChange::AddSecondary() Warning  ";
      G4cout << "theListOfSecondaries is full !! " << G4endl;
      G4cout << " The track is deleted " << G4endl;
    }
#endif
    G4Exception("G4VParticleChange::AddSecondary",
                "TRACK101", JustWarning,
                "Secondary Bug is full. The track is deleted");
  }
}


I see that geant4.9.5.p01 changed fSetSecondaryWeightByProcess back to be initialized to false; that is necessary, but is insufficient to fix this bug.


(I'm describing this rather than just attaching a patch file, because Geant4 developers need to look at this and consider the ramifications.)

　Sorry for late response. 
  I need time to discuss with developers concerned. 

  The solution proposed seems good
    http://bugzilla-geant4.kek.jp/show_bug.cgi?id=1283
  I've made a bug-fixed tag of track-V09-05-08.

  Modifications are following:
- In default (fSecondaryWeightByProcess flag is false),
  the weight of secondary tracks will be set to the parent weight,
  regardless of whether this process changed the parent's weight.
- If fSecondaryWeightByProcess flag is true,
  the weight of secondary tracks will not be changed
  by the ParticleChange
  (i.e. the process should set the secondary weight)
- Weight of Secondaries is set in AddSecondary
  (not in UpdateXXStep as before releases)
- Weight of parent track is set always
  if a new weight is proposed by process
  (i.e. isParentWeightSetByProcess is obsolete)
- Weight of parent track is set in accumulated manner same as
  other properties.
  i.e.) If two processes propose weight of W1 and W2 respectively
  for the track with initial weight of W0
  the final weight is set to  (W1/W0) * (W2/W0) * W0

Thank you very much for your help to point out and fix this problem.

Hisaya

I have just tried using geant4.9.5.p02; this is NOT fixed.

My regression tests still fail: secondary electrons and neutrinos from muon decay have weight=1 rather than the initial muon's weight.

I don't have time to investigate right now, and am staying with geant4.9.5.p01, with my fix.

The problem was fixed in 9.6 release