[PWGJE,PWGUD] adding upc selections to JE framework #14243
Conversation
nzardosh
requested review from
alibuild,
amatyja,
fjonasALICE,
maoyx,
mfasDa,
mhemmer-cern and
rolavick
as code owners
|
O2 linter results: ❌ 51 errors, |
github-actions
bot
changed the title
|
Dear UD experts, I have added a small modification to your isSelected function which allows one to optionally extract a vector with the amplitude values of the FIT detectors in bcs compatible with a UPC event. These can then be used later by analysers to make tighter event selections. However the functionality of the task should be unchanged by this addition |
nzardosh
force-pushed
the
upc
branch
2 times, most recently
from
2f81f28 to
edb13a4
Compare
nzardosh
changed the title
PWGJE/Core/JetDerivedDataUtilities.h
Outdated
| namespace jetderiveddatautilities | ||
| { | ||
|
|
||
| static constexpr float mPion = 0.139; // TDatabasePDG::Instance()->GetParticle(211)->Mass(); //can be removed when pion mass becomes default for unidentified tracks |
There was a problem hiding this comment.
i will fix it in a seperate PR
| Configurable<float> dcaZMax{"dcaZMax", 0.2, "maximum DCAZ selection for tracks - only applied for reassociation"}; | ||
| Configurable<int> upcBCRangeTimeWindow{"upcBCRangeTimeWindow", 1000, "time range in ns for bunch crossing range checking in upc gap determination"}; | ||
| Configurable<int> upcminNBCs{"upcminNBCs", 7, "minuimum number of bunch crossing to check in upc gap determination"}; | ||
| Configurable<int> upcMinNTracks{"upcMinNBC", 0, "minuimum number of tracks in upc event"}; |
There was a problem hiding this comment.
there is a copy paste error. The Laben should be "upcMinNTracks"
| if (collision.isCollisionSelected()) { | ||
|
|
||
| products.storedJCollisionsTable(collision.posX(), collision.posY(), collision.posZ(), collision.multFV0A(), collision.multFV0C(), collision.multFT0A(), collision.multFT0C(), collision.centFV0A(), collision.centFV0M(), collision.centFT0A(), collision.centFT0C(), collision.centFT0M(), collision.centFT0CVariant1(), collision.hadronicRate(), collision.trackOccupancyInTimeRange(), collision.alias_raw(), collision.eventSel(), collision.rct_raw(), collision.triggerSel()); | ||
| std::vector<float> amplitudesFV0; |
There was a problem hiding this comment.
vector creation and destruction in for each collision creates unnecessary heap allocation overhead. Consider instead something like:
// Outside loop
std::vector<float> amplitudesFV0;
// ... define others ...
for (auto const& collision : collisions) {
if (collision.isCollisionSelected()) {
amplitudesFV0.clear();
// ... clear others ...
// Copy data
auto amplitudesFV0Span = collision.amplitudesFV0();
amplitudesFV0.assign(amplitudesFV0Span.begin(), amplitudesFV0Span.end()); // More efficient than back_inserter
// ...
}
}There was a problem hiding this comment.
the problem with this is that the task is flirting with the 100 member limit as it is
There was a problem hiding this comment.
How is this related to members?
|
@nzardosh Don't waste CPU. |
|
Dear @nzardosh, the modification of the UD part looks ok to me, so I approve, but one question on the usage: Do you put some constraints on the bcrange? I wonder if one can hit a large range, and then the vectors would go large, which may cause some memory issues. |
Dear @rolavick thanks! If I understand correctly this upcCuts.SetNDtcoll(config.upcBCRangeTimeWindow); should set the range? or is there a better way you would recommend i can do it? thanks |
rolavick
left a comment
rolavick
left a comment
There was a problem hiding this comment.
Could you please incorporate this MegaLinter error
PWGUD/Core/SGSelector.h:65: Add #include for vector<> [build/include_what_you_use] [4]
thanks
5 participants
No description provided.