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

Adding fcsDataPresent flag #596

Merged
merged 7 commits into from
Dec 21, 2023
Merged

Conversation

akioogawa
Copy link
Contributor

Adding a flag to tell if FCS data was present or not.

Copy link
Contributor

@klendathu2k klendathu2k left a comment

Choose a reason for hiding this comment

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

Single change in StFcsCollection requested, otherwise good.

I am not reviewing anything in the pool directory... in fact... I suggest making the changes in the pool area a separate commit so that it is obvious that they have nothing to do with changing our event data model.

StRoot/StEvent/StFcsCollection.h Outdated Show resolved Hide resolved
@akioogawa
Copy link
Contributor Author

Many test failing with "St_db_Maker::LoadTable(TDataSet*): Assertion `!ee' failed."... I have no idea. Seems unrelated to my change of 1 line in StEvent...

@plexoos
Copy link
Member

plexoos commented Oct 27, 2023

Those failed jobs are tests with ROOT6. Unrelated to your changes. I restarted them and pretty sure they will pass without issues.

@akioogawa akioogawa requested a review from jdbrice November 1, 2023 13:39
@@ -64,9 +67,9 @@ class StFcsCollection : public StObject {
StSPtrVecFcsPoint mPoints[kFcsNDet];

Int_t mFcsReconstructionFlag=0; // undefined for now
Copy link
Member

Choose a reason for hiding this comment

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

Just a suggestion, maybe put the initialization in the constructor would be better.

@@ -154,7 +154,7 @@ Int_t StFcsRawDaqReader::Make() {
if(trgcmd != 4 && trgcmd !=10){ // 4=phys/ped 10=LED
nskip++;
nskiptot++;
if(mDebug) printf("trgcmd=%d skipping nskip=%d nskiptot=%d\n",trgcmd,nskip,nskiptot);
if(mDebug) LOG_INFO << Form("trgcmd=%d skipping nskip=%d nskiptot=%d",trgcmd,nskip,nskiptot)<<endm;
Copy link
Member

@zlchang zlchang Nov 1, 2023

Choose a reason for hiding this comment

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

Is there a need to introduce mDebug, maybe just use LOG_DEBUG<<Form(...)<<endm;?

@@ -330,7 +333,7 @@ Int_t StFcsRawDaqReader::Make() {
if(detid<6) nvaliddata++;

if(mDebug){
printf("FCS %3s : S%d:%d [det %d, ns %d, dep %d ch %d] det=%d id=%3d : size=%d : adc=",
printf("FCS %3s : S%2d:%2d [det%1d ns%1d dep%2d ch%2d] det=%d id=%3d size=%3d adc=",
Copy link
Member

Choose a reason for hiding this comment

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

Same as here, LOG_DEBUG?

@@ -353,7 +356,7 @@ Int_t StFcsRawDaqReader::Make() {
dd = mRdr->det("stgc")->get("altro",r) ;
while(dd && dd->iterate()) { //per xing and per RDO
if(mDebug) printf("STGC ALTRO: stgc%02d(sec) RDO=%1d ALTRO=%03d(row) Ch=%02d(pad)\n",dd->sec,r,dd->row,dd->pad);
for(uint32_t i=0; i<dd->ncontent; i++) {
for(u_int i=0; i<dd->ncontent; i++) {
if(mDebug) printf(" TB=%3d ADC=%4d",dd->adc[i].tb,dd->adc[i].adc) ;
Copy link
Member

@zlchang zlchang Nov 1, 2023

Choose a reason for hiding this comment

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

LOG_DEBUG and the same applies to a few lines below if(mDebug) printf("\n")?

@akioogawa akioogawa dismissed klendathu2k’s stale review December 21, 2023 19:54

Changed was accepted

@akioogawa akioogawa merged commit 8c67f7d into star-bnl:main Dec 21, 2023
148 checks passed
Copy link
Contributor

@klendathu2k klendathu2k left a comment

Choose a reason for hiding this comment

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

StEvent changes are fine. Generally I agree w/ Zilong's comments about LOG_DEBUG the mDebug parameter. But not a show stopper ... especially for a "pool" code.

dkapukchyan pushed a commit to dkapukchyan/star-sw that referenced this pull request Mar 11, 2024
Adding a flag to tell if FCS data was present or not.

---------

Co-authored-by: Akio Ogawa <[email protected]>
Co-authored-by: Akio Ogawa <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants