Skip to content

[EMCAL-729] Add compression of trigger bits#7774

Merged
shahor02 merged 1 commit intoAliceO2Group:devfrom
mfasDa:EMCAL-729
Dec 2, 2021
Merged

[EMCAL-729] Add compression of trigger bits#7774
shahor02 merged 1 commit intoAliceO2Group:devfrom
mfasDa:EMCAL-729

Conversation

@mfasDa
Copy link
Copy Markdown
Collaborator

@mfasDa mfasDa commented Nov 30, 2021

Reduction from 32 bit to 16 bit filtering the most
relevant triggers for EMCAL. For the moment
3 triggers are propagated

  • TF trigger
  • Physics trigger
  • Calib trigger
    In case other triggers will be included in the
    compressed output, they need to be appended.

Copy link
Copy Markdown
Collaborator

@shahor02 shahor02 left a comment

Choose a reason for hiding this comment

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

@mfasDa could you also fix the CTF unit test?

Comment on lines 159 to 160
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Reference for some reason seems to not work. The trigger bits stay unmodified. Will change it in the same way as done for the cells.

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.

you should use auto& currenttrigger = trigVec.emplace_back(ir, firstEntry, entries[itrig])

@mfasDa
Copy link
Copy Markdown
Collaborator Author

mfasDa commented Dec 1, 2021

@shahor02 OK, in fact besides missing adaption of the reference they were showed indeed a bug in the new decoding of compressed trigger bits. One question: Why do you prefer BOOST_CHECK with respect to BOOST_CHECK_EQUAL for the check for equal? If nothing speaks against I will change to BOOST_CHECK_EQUAL as this not only tells me that the test failed but why it failed.

Reduction from 32 bit to 16 bit filtering the most
relevant triggers for EMCAL. For the moment
2 triggers are propagated
- Physics trigger
- Calib trigger
In case other triggers will be included in the
compressed output, they need to be appended.

In addition the unit test is adapted (proper
reference, check for compressed and uncompressed
bits)
@mfasDa
Copy link
Copy Markdown
Collaborator Author

mfasDa commented Dec 1, 2021

In addition: Based on the discussion in the EMCAL meeting we drop the timeframe trigger from encoding.

@shahor02
Copy link
Copy Markdown
Collaborator

shahor02 commented Dec 1, 2021

If nothing speaks against I will change to BOOST_CHECK_EQUAL as this not only tells me that the test failed but why it failed.

you can change it if you prefer.

@shahor02
Copy link
Copy Markdown
Collaborator

shahor02 commented Dec 1, 2021

In addition: Based on the discussion in the EMCAL meeting we drop the timeframe trigger from encoding.

could you clarify what do you mean? If this is about the TF trigger bit of the RDH, you anyway don't have it, right?

@mfasDa
Copy link
Copy Markdown
Collaborator Author

mfasDa commented Dec 1, 2021

In addition: Based on the discussion in the EMCAL meeting we drop the timeframe trigger from encoding.

could you clarify what do you mean? If this is about the TF trigger bit of the RDH, you anyway don't have it, right?

Exactly, and that's why we drop it from being encoded (with respect to the initial version)

@mfasDa
Copy link
Copy Markdown
Collaborator Author

mfasDa commented Dec 2, 2021

@shahor02 should be ready for merge now?

@shahor02 shahor02 merged commit 637ffb3 into AliceO2Group:dev Dec 2, 2021
@mfasDa mfasDa deleted the EMCAL-729 branch December 2, 2021 10:04
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.

2 participants