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

Setting up Skimming Tasks - spectraTPC #5217

Merged
merged 2 commits into from Jan 20, 2021
Merged

Conversation

nzardosh
Copy link
Contributor

No description provided.

Copy link
Collaborator

@jgrosseo jgrosseo left a comment

Choose a reason for hiding this comment

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

Generally, we are ready to go despite my two comments below.
In addition:

  • please remove the _ from the file names
  • Let's also put the LFDerived.h into the SkimmingTutorials Folder (because it belongs there). This involves adding a library there and putting the dependency of the tasks to it

Analysis/Tasks/Skimming_Tutorials/spectraTPC_Provider.cxx Outdated Show resolved Hide resolved
Analysis/Tasks/Skimming_Tutorials/spectraTPC_Reference.cxx Outdated Show resolved Hide resolved
@nzardosh
Copy link
Contributor Author

Hi @shahor02 can you please check Common/MathUtils/include/MathUtils/detail/TypeTruncation.h that was added. Let me know if you think it would benefit from any modifications (inline declaration or making the float a template etc). thanks!

uint32_t iy;
} myu;
myu.y = x;
myu.iy &= mask;
Copy link
Collaborator

Choose a reason for hiding this comment

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

It's OK, thought it would be safer to have a protection against touching non-fraction bits, i.e.

constexpr uint32_t ProtMask = ((0x1<<9)-1)<<23;
myu.iy &= (ProtMask|mask);

Anyway, this can be done later.

@jgrosseo jgrosseo merged commit 6a055fd into AliceO2Group:dev Jan 20, 2021
EmilGorm pushed a commit to EmilGorm/AliceO2 that referenced this pull request Nov 22, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants