GRIFFINCollaboration / detectorSimulations_v10

Geant4 version 10 of the simulation code for the GRIFFIN array and it's suite of ancillary detection systems.
MIT License
9 stars 14 forks source link

Detector numbering incorrect in geant4.10.04 branch #65

Closed cnatzke closed 3 years ago

cnatzke commented 4 years ago

NTuple2EventTree cannot convert the output of the geant4.10.04 branch because the detectors are not being labeled properly. I generated a detector hit pattern using a clean build of the simulation directly from the ntuple via:

ntuple->Draw("detNumber","systemID<2000")

and have attached the output. detNumber1

There appear to be 17 detectors (0-16). The crystal mapping has the same issue where the simulation outputs 5 detectors rather than 4. Here is the macro I used to generate the events:

Co60.mac

Has anyone else had this issue?

cnatzke commented 4 years ago

After some testing the isssue seems to be turning on the suppressors with the command:

/DetSys/det/SetCustomShieldsPresent 1

in the run macro creates a spurious detector the simulation mistakes for detector 1.

cnatzke commented 4 years ago

If I generate a hit pattern with only one detector in position 5 and restrict the systemID to GRIFFIN Germanium I get the following:

ntuple->Draw("detNumber","systemID == 1000")

detNumber2 When the systemID includes BGO's I get:

ntuple->Draw("detNumber","systemID < 2000")

detNumber3.

Now adding two detectors (5 and 9):

ntuple->Draw("detNumber","systemID < 2000")

detNumber4

A new spurious detector appears in position 2. There seems to be a counter that increments the BGO detector number for every new clover position added. When all of the clovers are added this causes a detector to appear at bin 16 in the plot (meaning a detector at position 17) and this causes the error in NTuple2EventTree mentioned in the above reference.

Was this a design choice? Or a bug?

VinzenzBildstein commented 4 years ago

Most likely a bug. I'm not sure what the best way is to number the BGO detectors, but I would think that the BGO should have the same number as the GRIFFIN detector it surrounds?

cnatzke commented 4 years ago

That's how it's labeled in the array. I think we should stay as close to the array mnemonics as possible.

VinzenzBildstein commented 4 years ago

Agreed. That would mean we need to change how the names for the BGO volumes are created I guess?

cnatzke commented 4 years ago

I think so. The BGO detector number seems to be set by the line:

result.detectorNumber = static_cast<G4int>(ceil((assemblyNumber-5.)/13.));

at line 1038 in DetectorConstruction.cc

VinzenzBildstein commented 4 years ago

That line is probably right, it's more a question on how to set the assembly number. This might be impossible because it's auto-generated, but we got it work for GRIFFIN somehow so that the first detector isn't automatically 0 but whatever position it actually is in.

cnatzke commented 4 years ago
    if(volumeName.find("germaniumBlock1") != G4String::npos) {
        // strip "germaniumBlock1_" (16 characters) and everything before from the string
        std::string tmpString = volumeName.substr(volumeName.find("germaniumBlock1")+16);
        // replace all '_' with spaces so we can just use istringstream::operator>>
        std::replace(tmpString.begin(), tmpString.end(), '_', ' ');
        // create istringstream from the stripped and converted stream, and read detector and crystal number
        std::istringstream is(tmpString);
        is>>result.detectorNumber>>result.crystalNumber;
        // converting this number to a "true" detector number isn't necessary anymore since we use the real number and not the assembly/imprint number
        result.systemID = 1000;
        return result;
    }

Here is how we do it for the clovers. It looks like we parse the volume name and retrieve the detector number from there. I will look at the BGO volume names.

cnatzke commented 3 years ago

I think i found a fix for this issue. I will create a branch and submit a merge request.

cnatzke commented 3 years ago

This was fixed in pull request #69