Fixing pulser correction in production code #335
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…l setters from etofIn001 version of this file. Needed to unpack Get4Status bit into event files
…librations in the repository version. Nessecary only for creating calibrations, but should be available in the offical code. Added calculation of goodEventFlag for every counter from Get4 missmatch bits and pulser digis.
…librations in the repository version. Nessecary only for creating calibrations, but should be available in the offical code. Added calculation of goodEventFlag for every counter from Get4 missmatch bits and pulser digis.
…ect header version after rebase without increment
…base. Decremented etof collection ClassDef for real this time
|
Jason, do you have any issue with the proposed change in the geometry file? |
starsdong
reviewed
Mar 25, 2022
| @@ -1674,9 +1674,9 @@ StETofCalibMaker::calculatePulserOffsets( std::map< unsigned int, std::vector< u | |||
| double referenceTime = 0.; | |||
|
|
|||
| // update reference time (for QA) | |||
| if( pulserTimes.count( mReferencePulserIndex ) ) { | |||
| referenceTime = pulserTimes.at( mReferencePulserIndex ); //only updated for QA?? needed to remove smeared pulsers | |||
| if( mDoQA ) { | |||
There was a problem hiding this comment.
Philipp, maybe this line can be removed to simplify the code?
Question: does this affect the existing production data? e.g. Run20 FXT datasets which presumably have the ETOF calibrations in.
|
Hello Xin,
I should probably move the comment line two lines down to reduce confusion. The two line (+) that have been move outside the QA block definitely have to be where they are. Those lines not being processed without the QA flag on was the source of the bug.
Unfortunately, i missed last weeks software meeting to talk about this issue. Long story short, data so far would require to reproduction of the picoDSTs (at the moment, hit positions are completely of in the picoDSTs and track matching thus will be wrong), but i my suggestion would be to wait for us to do a bit more QA to make sure we don't have any other issues.
Best Regards
Philipp
March 25, 2022 6:46 PM, "Xin Dong" ***@***.*** ***@***.******@***.***>)> wrote:
@starsdong commented on this pull request.
------------------------------------
In StRoot/StETofCalibMaker/StETofCalibMaker.cxx (#335 (comment)):
@@ -1674,9 +1674,9 @@ StETofCalibMaker::calculatePulserOffsets( std::map< unsigned int, std::vector< u double referenceTime = 0.; // update reference time (for QA) + if( pulserTimes.count( mReferencePulserIndex ) ) { + referenceTime = pulserTimes.at( mReferencePulserIndex ); //only updated for QA?? needed to remove smeared pulsers if( mDoQA ) {
Philipp, maybe this line can be removed to simplify the code?
Question: does this affect the existing production data? e.g. Run20 FXT datasets which presumably have the ETOF calibrations in.
—
Reply to this email directly, view it on GitHub (#335 (review)), or unsubscribe (https://github.com/notifications/unsubscribe-auth/AIXRR2RISA753HWKIYLUDL3VBX3W3ANCNFSM5RRL3ZLA).
You are receiving this because you authored the thread.Message ID: ***@***.***>
|
|
Philipp, thanks. For the Run20 FXT data, can we run ETOF afterburner on MuDst data and reload the calibration to re-generate the picoDst or we need to process the whole chain from daq files? Thanks |
YannickSoehngen
approved these changes
Mar 28, 2022
|
Afterburner is enough. Even the currently uploaded calibrations are ok, it was just an on-the-fly correction that was not executed.
Best Regards
Philipp
March 28, 2022 12:05 AM, "Xin Dong" ***@***.*** ***@***.******@***.***>)> wrote:
Philipp, thanks. For the Run20 FXT data, can we run ETOF afterburner on MuDst data and reload the calibration to re-generate the picoDst or we need to process the whole chain from daq files? Thanks
—
Reply to this email directly, view it on GitHub (#335 (comment)), or unsubscribe (https://github.com/notifications/unsubscribe-auth/AIXRR2XU6DXHESO6SFQI4JLVCDLSNANCNFSM5RRL3ZLA).
You are receiving this because you authored the thread.Message ID: ***@***.***>
|
|
Thank you Philipp for the clarification. |
added 2 commits
April 6, 2022 12:47
…h data" This reverts commit 0f7aa47.
|
Hello everyone. I removed the changes to the geometry from this pull request. Once this is completed, i will submit the geometry changes in a separate pull request. I hope i did not mess everthing up in the progress :) |
starsdong
approved these changes
Apr 6, 2022
|
Looks good. Thanks Philipp. |
plexoos
approved these changes
Apr 6, 2022
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixing bug in production code that hid pulser correction behind QA flag in Calibmaker.
Pushing adjusted geometry xml to git.