Skip to content

Add calculation dX in TPC using local curvature#383

Merged
starsdong merged 1 commit intostar-bnl:mainfrom
fisyak:StidX
Sep 7, 2022
Merged

Add calculation dX in TPC using local curvature#383
starsdong merged 1 commit intostar-bnl:mainfrom
fisyak:StidX

Conversation

@fisyak
Copy link
Copy Markdown
Member

@fisyak fisyak commented Aug 12, 2022

No description provided.

@starsdong
Copy link
Copy Markdown
Member

Yuri, my understanding is that this request is to add a user function for dX calculation. Seem to be straightforward to me. Jason/Dmitri, any further comments?

@klendathu2k
Copy link
Copy Markdown
Contributor

Yuri, my understanding is that this request is to add a user function for dX calculation. Seem to be straightforward to me. Jason/Dmitri, any further comments?

To first order this looks fine. My only concern is whether there are other codes which write the dX value in StTpcHit? If so, what is the reason for replacing the calculation? (Or is the calculation just being moved into StiStEventFiller?)

@fisyak
Copy link
Copy Markdown
Member Author

fisyak commented Aug 13, 2022

As it was explained in message this method used local curvature for each hit instead of using curvature from the first and last hit. The only place where we can access local curvature (StKalanTrackNode parameters) is StiStEventFiller. This is essential for low pT tracks.
"

@starsdong
Copy link
Copy Markdown
Member

If no other objections, I would suggest to merge this PR. Thanks

Copy link
Copy Markdown
Member

@veprbl veprbl left a comment

Choose a reason for hiding this comment

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

Diff LGTM

@genevb
Copy link
Copy Markdown
Contributor

genevb commented Aug 15, 2022

Is the time required for this extra pair of pathlength calculations for every TPC hit on a track negligible?

node->getHelicity());
StThreeVectorD upper(tpcHit->positionU().x(),tpcHit->positionU().y(),tpcHit->positionU().z());
StThreeVectorD lower(tpcHit->positionL().x(),tpcHit->positionL().y(),tpcHit->positionL().z());
StThreeVectorD middle(tpcHit->position().x(),tpcHit->position().y(),tpcHit->position().z());
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 variable is unused

Suggested change
StThreeVectorD middle(tpcHit->position().x(),tpcHit->position().y(),tpcHit->position().z());

@klendathu2k
Copy link
Copy Markdown
Contributor

A quick grep shows that StTpcHit::setdX is called from StdEdxY2Maker.

I presume that dEdxY2 runs after StiStEventFiller. So the dX values will be overwritten when it runs. Don't we need to be able to shut this new calculation off in order to maintain the ability to reproduce old results?

@starsdong
Copy link
Copy Markdown
Member

Yuri, do you have any response to Jason's question? And regarding the time consumption raised by Gene, have you tested it also? Thanks

@plexoos plexoos added this to the SL22c milestone Aug 29, 2022
@starsdong starsdong merged commit 9b833bb into star-bnl:main Sep 7, 2022
@plexoos
Copy link
Copy Markdown
Member

plexoos commented Sep 7, 2022

Yuri, do you have any response to Jason's question? And regarding the time consumption raised by Gene, have you tested it also?

Was there any response to these questions?

There was also an unaddressed comment about a defined but unused variable in the submitted code. Why would we want to have it in the code? Doesn't it create a warning at compile time?

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.

6 participants