Skip to content

TPC: validity timestamp for time slot residual gain calibration#8523

Closed
badarots wants to merge 1 commit into
AliceO2Group:devfrom
badarots:dev
Closed

TPC: validity timestamp for time slot residual gain calibration#8523
badarots wants to merge 1 commit into
AliceO2Group:devfrom
badarots:dev

Conversation

@badarots
Copy link
Copy Markdown
Contributor

@badarots badarots commented Apr 7, 2022

Also, better default values for fit selection

@badarots badarots force-pushed the dev branch 2 times, most recently from a04ff14 to d7fe19d Compare April 8, 2022 07:37
Copy link
Copy Markdown
Collaborator

@wiechula wiechula left a comment

Choose a reason for hiding this comment

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

Just a minor suggestion.

Comment on lines 84 to 85
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

would either

Suggested change
static const double TFlength = 1e-3 * o2::raw::HBFUtils::Instance().getNOrbitsPerTF() * o2::constants::lhc::LHCOrbitMUS; // in ms
return static_cast<uint64_t>(processing_helpers::getCreationTime(pc) / TFlength);
static const auto TFlength = static_cast<uint64_t>(1e-3 * o2::raw::HBFUtils::Instance().getNOrbitsPerTF() * o2::constants::lhc::LHCOrbitMUS); // in ms
return processing_helpers::getCreationTime(pc) / TFlength;

or

Suggested change
static const double TFlength = 1e-3 * o2::raw::HBFUtils::Instance().getNOrbitsPerTF() * o2::constants::lhc::LHCOrbitMUS; // in ms
return static_cast<uint64_t>(processing_helpers::getCreationTime(pc) / TFlength);
static const double TFlength = 1e-3 * o2::raw::HBFUtils::Instance().getNOrbitsPerTF() * o2::constants::lhc::LHCOrbitMUS; // in ms
return static_cast<uint64_t>(double(processing_helpers::getCreationTime(pc)) / TFlength);

make sense?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

It makes, I would go for option 2. Option 1 will truncate the TFlengh from 11.4 ms to 11 ms.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Does this way of setting the validity time make sense for you? The drawback is that the validity time will not be exactly equal to the creation time, it will be the previous timestamp (in ms) multiple of the time frame interval... this is the same thing LHCClockCalibrator does.

The other option I could think of was to pass the time stamp as the TF identifier, which would set the validity to the exact creation timestamp, but this would change the semantics of CLI arguments like --tfs-per-slot to milliseconds per slot.

Also, better default values for fit selection
@github-actions
Copy link
Copy Markdown
Contributor

ghost commented May 21, 2022

This PR did not have any update in the last 30 days. Is it still needed? Unless further action in will be closed in 5 days.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Development

Successfully merging this pull request may close these issues.

2 participants