Skip to content

RHICf Sub-reconstruction tool (StRHICfPID)#519

Merged
plexoos merged 6 commits intostar-bnl:mainfrom
ggfdsa10:RHICfPID
Apr 19, 2023
Merged

RHICf Sub-reconstruction tool (StRHICfPID)#519
plexoos merged 6 commits intostar-bnl:mainfrom
ggfdsa10:RHICfPID

Conversation

@ggfdsa10
Copy link
Copy Markdown
Member

@ggfdsa10 ggfdsa10 commented Apr 2, 2023

RHICf Particle Identification reconstruction tool
this used in StRHICfPointMaker

1. Particle Identification reconstruction
Comment thread StRoot/StRHICfUtil/StRHICfPID.cxx Outdated
Comment on lines +29 to +32
float mPlateE[kRHICfNtower][kRHICfNplate];
float mPlateSumE[kRHICfNtower];
float mL20[kRHICfNtower];
float mL90[kRHICfNtower];
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This is not an IO class AFAICT. Why define these variables with single precision?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Hi Dmitri,

Thank you for your comments.
This class is a reconstructor for PID and not an IO class.
RHICf saving classes, such as StRHICfHit and StRHICfPoint, include mostly single-precision (float) variables.
StRHICfPID also receives input data as mPlateE from the StRHICfHit class using the setPlateEnergy() function.
Therefore, I didn’t see the need to change it to double precision.

However, I think calculation functions such as calculateEquation() could be changed to double precision.
Because that function needs to be more precise.
If you think it would be better to change these variables to double precision, I will make the changes along with others.

Thank you

Comment thread StRoot/StRHICfUtil/StRHICfPID.cxx Outdated
if(!mPlateEIs){return kRHICfFatal;}

for(int it=0; it<kRHICfNtower; it++){
for(int ip=0; ip<mPlateIdxNum; ip++){mPlateSumE[it] += layerSumEnergy(it, ip);}
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

See if you can use std::accumulate() here and in the next lines. It is likely to be more efficient than adding elements in a loop with if-statements...

Copy link
Copy Markdown
Member Author

@ggfdsa10 ggfdsa10 Apr 4, 2023

Choose a reason for hiding this comment

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

This line of code can be replaced with the following:

mPlateSumE[it] += (2*std::accumulate(mPlateE[it][0], mPlateE[it][9], 0));
mPlateSumE[it] += mPlateE[it][10];
mPlateSumE[it] += (2*mPlateE[it][11]);
mPlateSumE[it] += (4*std::accumulate(mPlateE[it][12], mPlateE[it][14], 0));
mPlateSumE[it] += (2*mPlateE[it][15]);

However, I’m not sure which version is better.
In my opinion, the original code lines are better than the replacement.
I think the original version seems more intuitive and simpler.

Comment thread StRoot/StRHICfUtil/StRHICfPID.cxx Outdated
ggfdsa10 added 2 commits April 4, 2023 03:58
2. change the function to std::fill() in init()
3. add init() in constructor
@ggfdsa10
Copy link
Copy Markdown
Member Author

ggfdsa10 commented Apr 4, 2023

All of the input data-checking functions for RHICf come from the StRHICfFunction class.
However, when someone uses a merged RHICf tool, those functions or codes will always be frozen.

Furthermore, all input data checkers simply ensure that the total number of input data is correct. For example, they check that the setPlateEnergy() function has been called as many times as there are plate energy arrays.

For this reason, I don’t think these checkers need to be included in all of the RHICf tools, so I plan to remove them from all RHICf tools soon.
Commitd92b32d was made for the same reason.

@starsdong
Copy link
Copy Markdown
Member

I think this one is ready to merge. Dmitri, please go head. Thanks

@plexoos plexoos merged commit 97d8d77 into star-bnl:main Apr 19, 2023
plexoos pushed a commit that referenced this pull request May 19, 2023
all of the checking function is removed
these functions are NOT necessary to RHICf reconstruction. 
In more detail, see the comment below,  (in PR #519 )


-----------------------------------------------------------------------------------------------------------------------
> All of the input data-checking functions for RHICf come from the
StRHICfFunction class. However, when someone uses a merged RHICf tool,
those functions or codes will always be frozen.
> 
> Furthermore, all input data checkers simply ensure that the total
number of input data is correct. For example, they check that the
setPlateEnergy() function has been called as many times as there are
plate energy arrays.
> 
> For this reason, I don’t think these checkers need to be included in
all of the RHICf tools, so I plan to remove them from all RHICf tools
soon.
Commit[d92b32d](d92b32d)
was made for the same reason.

-------------------------------------------------------------------------------------------------------------------------
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.

3 participants