calo PID for TTracks

@amerli In future, when pulling updates from master into your feature branches please do NOT merge the changes, always rebase. merging can cause problems, such as right now where your branch cannot be easily rebased. Always rebase when pulling updates with

> git pull --rebase --autostash origin master

@jonrob Thanks for letting me know, I will do it

@amerli There are issues in the pipeline builds, please take a look.

assigned to @jonrob

assigned to @amerli and unassigned @jonrob

@jonrob The build works both locally and in the ci-test here https://lhcb-nightlies.web.cern.ch//nightly/lhcb-master-mr/12727/. How is the build done in the pipeline? The issue in the pipeline (https://gitlab.cern.ch/lhcb/Moore/-/jobs/52523309#L50) is due to CaloChargedPIDsAlg_v1 which is a new class added in this MR. Could this be the reason of the failure?

CaloChargedPIDsAlg_v1 is not added in this MR, its added in Rec!4233

But yes, thats the problem as a limitation of the pipeline builds is they do not known about dependent MRs. So the failure is expected.

Ah yes my fault, it has been added in Rec. I did it sometime ago but I forgot the details

The issue for the failure in the pipeline builds is understood. @jonrob if you don't have anything else to add and you're satisfied, can you resolve the thread?

mentioned in merge request !4519 (merged)

mentioned in issue #936

@tfulghes, @msaur, @jonrob, can I please request an update for the timeline on this being tested and hopefully merged?

The QEE internal deadline for HLT2 lines is noon this Friday (Friday 21st March) and we need TTrack calo PID for a new line for HNL -> mu e nu Moore!4519. There are also benefits to HNL -> mu pi (Moore!4129) and Dark Higgs -> mu mu (Moore!4463).

If testing this merge request is not likely to be done towards the middle of this week, is there any chance of flagging it as higher priority?

Hi @amerli, @scollavi could you please rebase this and Rec MRs, so I can start to test them

@tfulghes Master merged into this branch

/ci-test Rec!4233

Started integration test build. Once done, check the results or the comparison to a reference build.

@amerli General comment, never merge master into your branch, it can create a mess. Always use rebase.

@msaur thanks for the comment, as suggested by Chris next time I will use

git pull --rebase --autostash origin master

for this MR I already merged master in the past. If needed I can try to manually rebase each commit.

@tfulghes it's good also to merge and test LHCb!4901. It's not reported in the description because it doesn't strictly depend on this MR

All the failures seem related to the change of the algorithm name, so we probably need a ref update, and it is fine. Do you want to retest also with LHCb MR, or could it go in afterwards @amerli ?

@tfulghes if afterwards means in some days, fine with it. Otherwise it's probably better to merge everything together because they are correlated

If all MRs are part of one development then they should go together.

Throughput Test MooreOnline_hlt2_pp_2024_data: 170.0 Events/s -- change of -0.76% vs. reference

I agree with you @msaur. @tfulghes can you start the test with this MRs together with Rec!4233 and LHCb!4901?

/ci-test Rec!4233 LHCb!4901

Started integration test build. Once done, check the results or the comparison to a reference build.

Throughput Test MooreOnline_hlt2_pp_2024_data: 170.2 Events/s -- change of -0.12% vs. reference

added 44 commits

eb052e7f...29dde2a9 - 43 commits from branch master
58a11c6f - Merge branch 'master' into amerli_caloreco4ttracks

Compare with previous version

added 3 commits

58a11c6f...ab6e46a5 - 2 commits from branch master
02dfffe9 - Merge branch 'master' into amerli_caloreco4ttracks

Compare with previous version

changed milestone to %RTA 2025 Data taking

/ci-test LHCb!4901 Rec!4233

mentioned in merge request !4463 (merged)

mentioned in commit d95eaf07

added 1 commit

c0784c2a - check shared_container if Shared in location of DataHandle; removed correlated with statements

Compare with previous version

added 1 commit

9c4f52d5 - pre-commit fixes

Compare with previous version

added 19 commits

9c4f52d5...651db7d0 - 18 commits from branch master
28b25047 - Merge branch 'master' into amerli_caloreco4ttracks

Compare with previous version

Hi @amerli, I see some commits after the tests. Is it ready to be re-tested?

Yes, I see @msaur has already started the test

approved this merge request

/ci-test LHCb!4901 Rec!4233

Started integration test build. Once done, check the results or the comparison to a reference build.

@msaur I don't see any major issue apart from the fact that a ref update is needed. Can this be merged?

Hello @msaur, could I ask for an estimate of when this (and its partner in Rec) might reach the end of the queue for merging please?

We have a dependent selection MR in QEE that is held up by this which we need to get in by Friday (but ideally much sooner) before we undraft our QEE merge request.

Many thanks, your hard work is appreciated.

@rjhunter Good morning, this is relatively lower on the to-do list and developments with global impact have priority. For QEE MR you could list this set of MRs as a required dependency (or directly base your branch on top of this one but then you would need to keep everything updated all the time).

Thanks @msaur, OK we'll merge and indeed make this a dependent MR.

mentioned in commit bfd317b4

mentioned in commit 1a911c44

mentioned in merge request !4367

mentioned in commit e4c5eca9

calo PID for TTracks

Merge request reports

Activity