Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

added charge scale factor in channel status #206

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

rl23
Copy link

@rl23 rl23 commented Nov 21, 2024

No description provided.

@@ -544,6 +543,7 @@ void Gsim::MakeEvent(const G4Event *g4ev, DS::Root *ds) {
int numPE = 0;

for (int ipmt = 0; ipmt < hitpmts->GetEntries(); ipmt++) {
double chargeScale = 1;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we move this down to l. 558? I believe GetChannelStatus should set the default value via the ratdb entry you added.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done :)

@tannerbk tannerbk self-assigned this Dec 2, 2024
@tannerbk tannerbk self-requested a review December 2, 2024 18:12
Copy link
Contributor

@tannerbk tannerbk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @rl23, I've added some small comments.

@@ -12,3 +12,9 @@
"run_range": [0, 0],
"default_value": 1,
}
{
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you add a new line here to separate the tables? I think it would make it slightly easier to read.

}
}
}
mc->SetNumPE(numPE);
}

void Gsim::AddMCPhoton(DS::MCPMT *rat_mcpmt, const GLG4HitPhoton *photon, EventInfo * /*exinfo*/, std::string process) {
void Gsim::AddMCPhoton(DS::MCPMT *rat_mcpmt, const GLG4HitPhoton *photon, EventInfo * /*exinfo*/, std::string process,
double chargeScale) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think this should be a new line.

@@ -592,7 +592,7 @@ void Gsim::AddMCPhoton(DS::MCPMT *rat_mcpmt, const GLG4HitPhoton *photon, EventI
rat_mcphoton->SetCreationTime(photon->GetCreationTime());

rat_mcphoton->SetFrontEndTime(fPMTTime[fPMTInfo->GetModel(rat_mcpmt->GetID())]->PickTime(photon->GetTime()));
rat_mcphoton->SetCharge(fPMTCharge[fPMTInfo->GetModel(rat_mcpmt->GetID())]->PickCharge());
rat_mcphoton->SetCharge(chargeScale * fPMTCharge[fPMTInfo->GetModel(rat_mcpmt->GetID())]->PickCharge());
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you add a comment here to clarify what "chargeScale" is doing? For example, "Set the charge for the photoelectron, scaled by an optional calibration parameter 'chargeScale' that is default 1.0".

@llebanowski
Copy link
Contributor

It is probably worth adding this parameter to the meta info in src/io/src/OutNtupleProc.* like the other ChannelStatus parameters.

It is interesting that the channel charge is implemented in Gsim - is this the only or best place?

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants