-
Notifications
You must be signed in to change notification settings - Fork 62
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
TriggerSimu + picoDst for BEMC data in daq->picoDst production #314
Conversation
starsdong
commented
Feb 28, 2022
- PicoDst EMC collection pointer taken from StEvent directly in case of daq production
- TriggerSimuMaker added to BFC chain
- EmcRawMaker used to obtain BEMC data in case of daq production
…of daq production - TriggerSimuMaker added to BFC chain - EmcRawMaker used to obtain BEMC data in case of daq production
This line is trying to deal with the daq->picoDst production where StEvent should be there. In the MuDst->picoDst production, the EmcCollection will be obtained from MuDst directly (line 861). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't see any show stoppers. So approving, with the following observation--
The changes in the picoDst add an additional path to accessing the EMC collection. This path is only enabled in production codes (not in the TFG variant).
|
||
// | ||
if ( maker == "StEmcRawMaker" && GetOption("picoWrite")) { | ||
mk->SetMode(1); // saveAllStEvent for picoDst |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just documenting here, since I didn't know about this before, that this mode enables some additional hit-saving in StEmcRawMaker.cxx:
if((m_Mode&0x1)==1)
{
LOG_INFO << "Setting BEMC debug Mode -> save all hits into StEvent"<<endm;
mBemcRaw->saveAllStEvent(kTRUE);
mBemcRaw->initQAHisto();
}
-Gene
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The BFC-related changes, and use of StMaker::SetMode(), look OK to me.
Hi Dmitri, may need your help here. One of the tests from the CI build failed (119). But I tested the same macro/chain which ran OK with my local setup. See log file /star/u/dongx/pwg/Git/20220225/test_21.log |
By comparing the log file, it seems the test job crashed in TriggerSimuMaker Eemc related part when trying to load some local files |
Xin, in your log file I see the following lines which may suggest that the job is trying to access non existing local files. If this is the case, and the files are indeed used they should be placed in the repository or database.
|
Hi guys,
This is a known issue. We had thought about put these masks into database. For this task, we need to creat new table structure in the database. I believe I have all the information from EEMC expert regarding the masks. This was a low priority task. I was supposed to do the work, but the work got stalled for a year or so. The issue will prevent you from running jobs in nodes that do not have access to afs file system.
Zilong
On Feb 28, 2022, at 4:13 PM, Dmitri Smirnov ***@***.***> wrote:
Xin, in your log file I see the following lines which may suggest that the job is trying to access non existing local files. If this is the case, and the files are indeed used they should be placed in the repository or database.
3491 StTriggerSimuMaker:INFO - ^[[32mEemc::InitRun() yyyymmdd=20210624 hhmmss=065455
3492 ^[[0m
3493 StTriggerSimuMaker:INFO - ^[[32mUsing mask file /star/u/pibero/public/StarTrigSimuSetup/mask/eec.05-13-09.dat^[[0m
3494 StTriggerSimuMaker:INFO - ^[[32mUsing mask directory /star/u/pibero/public/StarTrigSimuSetup/mask/10.15.2010^[[0m
3495 StTriggerSimuMaker:INFO - ^[[32mScanning mask file /star/u/pibero/public/StarTrigSimuSetup/mask/10.15.2010/tower-1-current_beam_config.dat^[[0m
3496 StTriggerSimuMaker:INFO - ^[[32mScanning mask file /star/u/pibero/public/StarTrigSimuSetup/mask/10.15.2010/tower-2-current_beam_config.dat^[[0m
3497 StTriggerSimuMaker:INFO - ^[[32mScanning mask file /star/u/pibero/public/StarTrigSimuSetup/mask/10.15.2010/tower-3-current_beam_config.dat^[[0m
3498 StTriggerSimuMaker:INFO - ^[[32mScanning mask file /star/u/pibero/public/StarTrigSimuSetup/mask/10.15.2010/tower-4-current_beam_config.dat^[[0m
3499 StTriggerSimuMaker:INFO - ^[[32mScanning mask file /star/u/pibero/public/StarTrigSimuSetup/mask/10.15.2010/tower-5-current_beam_config.dat^[[0m
3500 StTriggerSimuMaker:INFO - ^[[32mScanning mask file /star/u/pibero/public/StarTrigSimuSetup/mask/10.15.2010/tower-6-current_beam_config.dat^[[0m
3501 StTriggerSimuMaker:INFO - ^[[32mUsing EEMC OFFLINE pedestals^[[0m
—
Reply to this email directly, view it on GitHub<#314 (comment)>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/AGH7F3JWLVZSLDXYPWCTH4DU5PQPDANCNFSM5PRZ3ZRQ>.
You are receiving this because your review was requested.Message ID: ***@***.***>
|
I see similar lines shown in the production log file for MuDst->picoDst conversion. |
This was happening without us (the Production Team) knowing about it. Of course it isn't OK to access files that are in some private directory because such a directory won't be available to all possibilities for running the job. It seems we've been "lucky" in some sense for the past couple years to be running all these conversion productions on the RACF/SDCC farm.
…-Gene
On Feb 28, 2022, at 5:02 PM, Xin Dong ***@***.******@***.***>> wrote:
I see similar lines shown in the production log file for MuDst->picoDst conversion.
/star/rcf/prodlog/P20ic.SL21d/log/daq/job_st_physics_adc_18105030_raw_5500050.log.gz
Gene, how did we handle it in the current production?
|
|
Hi Xin,
I can try to look into this. The impact is very minimal. If I remember correctly, for run12 and run13, there should be no impact at all whether you read or don’t read those files.
Zilong
On Feb 28, 2022, at 5:25 PM, Xin Dong ***@***.***> wrote:
1. Zilong, if you can help with the DB structure and this is not a big amount of work, would be very much appreciated if you can help.
2. However, I am also wondering how useful these tables are. It seems all jobs are reading the same mask files which were created in 2010. Are they just for test or indeed can be valid still for current data?
—
Reply to this email directly, view it on GitHub<#314 (comment)>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/AGH7F3MZTKSG3SKL5PB652LU5PY33ANCNFSM5PRZ3ZRQ>.
You are receiving this because your review was requested.Message ID: ***@***.***>
|
Zilong, do you have any update on this? Would be great if you have some findings on the difference, so maybe tomorrow we can decide the path forward. |
Just a reminder that the isobar st_hf dataset has already been processed into MuDsts, and only needs the conversion to picoDsts. I don't need to wait for SL22a for that second step. Just the database content is the concern for that, I guess, and since I'll run it on RACF/SDCC, there will be access to those private files just as has been the case for conversion productions over the past few years (though now I'm wondering if there were any issues for embedding that produced picoDsts at other sites or in containers without AFS).
…-Gene
On Mar 1, 2022, at 5:10 PM, Xin Dong ***@***.******@***.***>> wrote:
Zilong, do you have any update on this? Would be great if you have some findings on the difference, so maybe tomorrow we can decide the path forward.
Specially on the st_hf isobar data production, I believe what matters are only the BHT triggers. These eemc masks will not matter. These masks may have an impact for pp picoDst production as it involves some JP triggers.
|
assert(0); | ||
} | ||
mTables = mAdc2e->getBemcData()->getTables(); | ||
if(mAdc2e) mTables = mAdc2e->getBemcData()->getTables(); | ||
else if(emcRaw) mTables = emcRaw->getBemcRaw()->getTables(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
else if(emcRaw) mTables = emcRaw->getBemcRaw()->getTables(); | |
else if(emcRaw) mTables = emcRaw->getBemcRaw()->getTables(); | |
else{ | |
LOG_FATAL << "StBemcTriggerSimu couldn't find StEmcADCtoEMaker or StEmcRawMaker in chain" << endm; | |
assert(0); | |
} |
if(!mAdc2e) { | ||
LOG_FATAL << "StBemcTriggerSimu couldn't find StEmcADCtoEMaker in chain" << endm; | ||
StEmcRawMaker *emcRaw = static_cast<StEmcRawMaker*> ( mHeadMaker->GetMakerInheritsFrom("StEmcRawMaker") ); | ||
if(!mAdc2e && !emcRaw) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if(!mAdc2e && !emcRaw) { |
LOG_FATAL << "StBemcTriggerSimu couldn't find StEmcADCtoEMaker in chain" << endm; | ||
StEmcRawMaker *emcRaw = static_cast<StEmcRawMaker*> ( mHeadMaker->GetMakerInheritsFrom("StEmcRawMaker") ); | ||
if(!mAdc2e && !emcRaw) { | ||
LOG_FATAL << "StBemcTriggerSimu couldn't find StEmcADCtoEMaker and StEmcRawMaker in chain" << endm; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LOG_FATAL << "StBemcTriggerSimu couldn't find StEmcADCtoEMaker and StEmcRawMaker in chain" << endm; | |
LOG_FATAL << "StBemcTriggerSimu couldn't find StEmcADCtoEMaker in chain" << endm; | ||
StEmcRawMaker *emcRaw = static_cast<StEmcRawMaker*> ( mHeadMaker->GetMakerInheritsFrom("StEmcRawMaker") ); | ||
if(!mAdc2e && !emcRaw) { | ||
LOG_FATAL << "StBemcTriggerSimu couldn't find StEmcADCtoEMaker and StEmcRawMaker in chain" << endm; | ||
assert(0); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
assert(0); | |
LOG_FATAL << "StBemcTriggerSimu couldn't find StEmcADCtoEMaker in chain" << endm; | ||
StEmcRawMaker *emcRaw = static_cast<StEmcRawMaker*> ( mHeadMaker->GetMakerInheritsFrom("StEmcRawMaker") ); | ||
if(!mAdc2e && !emcRaw) { | ||
LOG_FATAL << "StBemcTriggerSimu couldn't find StEmcADCtoEMaker and StEmcRawMaker in chain" << endm; | ||
assert(0); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
} | |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I suggested a few cosmetic changes to code, please take a look at and see if this okay with you guys
Thank you Zilong. I made the suggested cleanup in the new commit. A quick test with the Run22 data looks OK. |
…cluster jobs, DB solution being worked on) - PicoDstMaker EMC trigger threshold check updated to include lower thresholds
I made two updates (Eemc local files for masking skipped - they are actually not in effect when running on clusters). Zilong is working on the DB solution as discussed on Wed. |
Looks good to me. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks okay.
A note on some results from nightly tests since this PR was merged: this has resulted in very significant increases to the size of picoDst, MuDsts, and even event.root files for many of the jobs. I'm particularly concerned about the MuDst increases which are larger than a factor of x2!!! See for example these two nightly tests' DSTs:
-rw-r--r-- 1 starreco stargrid 549987604 Mar 5 03:38 /star/rcf/test/dev/daq_sl302.ittf_opt/Sat/year_2016/dAu20_production_2016/st_physics_17155014_raw_3000009.event.root
-rw-r--r-- 1 starreco rhstar 48308496 Mar 5 04:18 /star/rcf/test/dev/daq_sl302.stica/Sat/year_2021/production_OO_200GeV_2021/st_physics_22134030_raw_4500015.MuDst.root |
I wonder the size increase is probably due to the StEmcRawMaker::SetMode(1) change to enable TriggerSimuMaker working in picoDst production. I can see the increase in picoDst is primarily due to the new branches EmcTrigger and EmcPidTraits got filled. Zilong, if possible, could you please help check whether we can identify a way to keep the same StEvent/MuDst structure while allowing the TriggerSimuMaker to work OK with EmcCollection retrieved from EmcRawMaker? |
Hi Xin,
I’m a bit surprised that this is due to setting the saveALLStEvent to true in the StEmcRawMaker. The size should be same, unless there are some weird things in the offline table for BSMD and preshowers. I don’t think the gain and pedestals for these detectors are maintained.
We can hack the code so that it won’t save information for BSMD or preshowers. But this is a dangerous thing to do.
Zilong
On Mar 11, 2022, at 5:18 PM, Xin Dong ***@***.***> wrote:
I wonder the size increase is probably due to the StEmcRawMaker::SetMode(1) change to enable TriggerSimuMaker working in picoDst production.
I can see the increase in picoDst is primarily due to the new branches EmcTrigger and EmcPidTraits got filled.
In MuDst, I see new branches are there: EmcPrs (size of 4800), EmcSmde (size 18000) and EmcSmdp (size 18000).
Zilong, if possible, could you please help check whether we can identify a way to keep the same StEvent/MuDst structure while allowing the TriggerSimuMaker to work OK with EmcCollection retrieved from EmcRawMaker?
—
Reply to this email directly, view it on GitHub<#314 (comment)>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/AGH7F3KSQFTZXPR62TSGEUTU7PBDLANCNFSM5PRZ3ZRQ>.
You are receiving this because your review was requested.Message ID: ***@***.***>
|
I have another request, @zlchang : StTriggerSimuMaker prints a large number of lines of text to the log files (something like 7000 lines of text on the first event). Is all of this text output helpful for every job? I encountered this because some of the lines of text include the word "abort" as part of some trigger name (which I realize is legitimate such as when related to the collider's "abort gap"), and that was tripping up our job-failure detection scripts. We can make the scripts smarter about this, but do all these trigger items really need to be printed? Thanks, |
Hi Gene, It prints out the status and pedestals of barrel and Endcap towers, so they're long. If you prefer, I can make them to be printed when only in debug mode. But I don't see the word "abort" is printed for trigger simulator? Do you have a concrete example? Zilong |
Concrete example:
All of these nightly test datasets showed this, so perhaps it was only used as a name in 2016 and 2017: AuAu200_production_2016 -Gene |