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

Updating Stub Format for VMSMERouter Changes #279

Merged
merged 24 commits into from
Jun 11, 2024

Conversation

mlarson02
Copy link

This PR updates the DTC emulation and Track Finding emulation Disk Stub data format by adding to the beginning of the stubword a bit determining whether the stub is in the negative z region of the detector, and adding an offset to the encoded r value for DiskPS stubs of 256 (7.5 cm) to allow fitting the negDisk bit in 36 bits.

This information is required in the AllStubs to be used for the new VMSMERouter dual FPGA module, and it was decided this was the easiest way to incorporate the information without making changes to the previous processing modules / causing the DTC/Input/AllStubs to be of different format. More information regarding this is available here.

PR also includes changes to written memory text files required for current master firmware-hls (writing offset r values for DiskPS stubs and negDisk bit for all disk stubs).

tomalin and others added 20 commits March 23, 2024 21:09
* Add z0 resolution to performance printout

* code format
* Disable binning in DR

* bug fix

* add comment

* code format

* tweak comment

* fix previous erroneous commit
* Make combined modules default

* tweak

* Improve USEHYBRID ifdef range

* Fix compiler error for pure Tracklet algo

* Move fitpattern.txt refs, so only used for pure Tracklet algo

* code format
* Made calculations more numerically stable.

* Explicitly restrict to domain of asin/acos.

* Added comments.

* Code format.

* Fixed typos in comments.

* Only do calculations when needed.

* Added const where applicable.
* Fix inventStubs bug in duplicate removal

When using L2L3D1 seeds, it is only L2L3 that are used to determined the r-z helix params, not L2D1 as the code currently assumes.

* Update PurgeDuplicate.cc

* add comment

* formatted
Full agreement with HLS (100 events L1PHIC - D5PHIC)
@mlarson02 mlarson02 marked this pull request as ready for review May 21, 2024 20:27
@@ -551,7 +551,7 @@ namespace tmtt {
matRinv = TMatrixD(TMatrixD::kInverted, matR);
} else {
// Protection against rare maths instability.
const TMatrixD unitMatrix(TMatrixD::kUnit, TMatrixD(nHelixPar_, nHelixPar_));
const TMatrixD unitMatrix(TMatrixD::kUnit, TMatrixD(2, 2));
Copy link

Choose a reason for hiding this comment

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

Is this related to these changes?

@@ -1,4 +1,4 @@
To run the L1 tracking & create a TTree of tracking performance:
To run the L1 tracking & create a TTree of tracking performance:
Copy link

Choose a reason for hiding this comment

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

Remove extra space

@@ -344,7 +345,7 @@ namespace trklet {

double kz() const { return 2.0 * zlength_ / (1 << nzbitsstub_[0]); }
double kz(unsigned int layerdisk) const { return 2.0 * zlength_ / (1 << nzbitsstub_[layerdisk]); }
double kr() const { return rmaxdisk_ / (1 << nrbitsstub_[N_LAYER]); }
double kr() const { return rmaxdisk_ / (1 << (nrbitsstub_[N_LAYER] + 1)); }
Copy link

Choose a reason for hiding this comment

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

Can you add a comment to explain?

//Following values are used for duplicate removal
//Rinv bins were optimised to ensure a similar number of tracks in each bin prior to DR
//Rinv bin edges for 6 bins.
std::vector<double> rinvBins_{-rinvcut(), -0.004968, -0.003828, 0, 0.003828, 0.004968, rinvcut()};
//std::vector<double> rinvBins_{-rinvcut(), -0.004968, -0.003828, 0, 0.003828, 0.004968, rinvcut()};
Copy link

Choose a reason for hiding this comment

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

Are these changes related to this updated?

@@ -221,6 +221,8 @@ namespace trklet {

unsigned int PSseed() const { return ((layer() == 1) || (layer() == 2) || (disk() != 0)) ? 1 : 0; }

// Seed type:
Copy link

Choose a reason for hiding this comment

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

Why is this added?

@@ -160,7 +164,7 @@ double Stub::rapprox() const {
else
return settings_.rDSSouter(r_.value());
}
return r_.value() * settings_.kr();
return (r_.value() + (1 << 8)) * settings_.kr(); // Incorporating r offset for disk PS stubs
Copy link

Choose a reason for hiding this comment

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

Can you do something like "int r_offset_bits=8" and use this variable?

@@ -403,9 +403,9 @@ void TrackletProcessor::execute(unsigned int iSector, double phimin, double phim
if (negdisk) {
indexz = ((1 << nbitszfinebintable_) - 1) - indexz;
}
indexr = stub->r().value() >> (stub->r().nbits() - nbitsrfinebintable_);
indexr = stub->rvalue() >> (stub->r().nbits() + 1 - nbitsrfinebintable_);
Copy link

Choose a reason for hiding this comment

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

Can you add comment about offset of +1

Copy link

@aryd aryd left a comment

Choose a reason for hiding this comment

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

I added a few questions and comments on the updates.

@mlarson02
Copy link
Author

I added a few questions and comments on the updates.

I've addressed the comments relating to my changes, the others are regarding code included in CMSSW 14 dev branch commits incorporated in my branch, but not yet merged into the ProjectionCalculator_without_IMATH_and_noncombined branch.

@aryd aryd merged commit ab2f3fe into ProjectionCalculator_without_IMATH_and_noncombined Jun 11, 2024
1 check failed
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