Sweeping !20881 from 21.3 to 21.9. Allow selection on ET/PT of Offline objects
Allow selection on ET/PT of Offline objects
See merge request !20881 (merged)
Merge request reports
Activity
This merge request affects 2 packages:
- Trigger/TrigAnalysis/TrigInDetAnalysis
- Trigger/TrigAnalysis/TrigInDetAnalysisUser
Adding @sutt as watcher
added 21.9 Trigger review-pending-level-1 labels
-
Athena externals 
cmake N/A make N/A required tests N/A optional tests N/A 
-
This merge request affects 2 packages:
- Trigger/TrigAnalysis/TrigInDetAnalysis
- Trigger/TrigAnalysis/TrigInDetAnalysisUser
Adding @sutt as watcher
-
Athena externals cmake N/A make N/A required tests N/A optional tests N/A
-
This merge request affects 2 packages:
- Trigger/TrigAnalysis/TrigInDetAnalysis
- Trigger/TrigAnalysis/TrigInDetAnalysisUser
Adding @sutt as watcher
-
Athena externals 
cmake make required tests optional tests 
Full details available at NICOS MR-20918-2019-02-08-16-39
added review-user-action-required label
removed review-pending-level-1 label
Hi @sutt, how should we proceed with this one?
Tadej (L2)
I don't understand the issue, what exactly is the problem ? If it is the failures of the tests, then these are completely irrelevant, as these "required" tests don't even run the code that was changed.
I will not be making any updates, since the changes are only simple, and in any case, there would be no point, since they would not affect any of the output from the tests, which do not even run this code.
For information, the code in TrigInDetAnalysisUser, builds standalone executables, that no athena code, or athena tests ever run, and they are used only for out ART validation and out offline performance evaluation, so as long as the code builds ok, that is all that should be needed for these changes.
Mark
-
This merge request affects 2 packages:
- Trigger/TrigAnalysis/TrigInDetAnalysis
- Trigger/TrigAnalysis/TrigInDetAnalysisUser
Adding @sutt as watcher
added review-pending-level-1 label and removed review-user-action-required label
-
Athena externals cmake make required tests optional tests Full details available at NICOS MR-20918-2019-04-18-19-43
For reference, in TrigInDetAnalysisUser build logfile we see:
Warning: the last line does not indicate that the package build is successfulIn TrigInDetAnalysisExample build logfile:
n file included from /var/lib/jenkins/workspace/CI-MERGE-REQUEST/21.9/Trigger/TrigAnalysis/TrigInDetAnalysis/TrigInDetAnalysis/TrackAnalysis.h:26:0, from /var/lib/jenkins/workspace/CI-MERGE-REQUEST/21.9/Trigger/TrigAnalysis/TrigInDetAnalysisExample/TrigInDetAnalysisExample/Analysis_Distribution.h:20, from /var/lib/jenkins/workspace/CI-MERGE-REQUEST/21.9/Trigger/TrigAnalysis/TrigInDetAnalysisExample/src/Analysis_Distribution.cxx:15: /var/lib/jenkins/workspace/CI-MERGE-REQUEST/21.9/Trigger/TrigAnalysis/TrigInDetAnalysis/TrigInDetAnalysis/TrigObjectMatcher.h:57:3: error: 'TrigObjectMatcher::TrigObjectMatcher(TrackSelector*, const std::vector<TrackTrigObject>&, bool (*)(const TrackTrigObject&, TIDA::Track*))' cannot be overloadedIn TrigIDJpsiMonitoring build logfile:
In file included from /var/lib/jenkins/workspace/CI-MERGE-REQUEST/21.9/Trigger/TrigAnalysis/TrigInDetAnalysis/TrigInDetAnalysis/TrackAnalysis.h:26:0, from /var/lib/jenkins/workspace/CI-MERGE-REQUEST/21.9/Trigger/TrigAnalysis/TrigInDetAnalysisUtils/TrigInDetAnalysisUtils/T_AnalysisConfig.h:31, from /var/lib/jenkins/workspace/CI-MERGE-REQUEST/21.9/Trigger/TrigMonitoring/TrigIDJpsiMonitoring/TrigIDJpsiMonitoring/AnalysisConfig_Jpsi.h:18, from /var/lib/jenkins/workspace/CI-MERGE-REQUEST/21.9/Trigger/TrigMonitoring/TrigIDJpsiMonitoring/src/AnalysisConfig_Jpsi.cxx:30: /var/lib/jenkins/workspace/CI-MERGE-REQUEST/21.9/Trigger/TrigAnalysis/TrigInDetAnalysis/TrigInDetAnalysis/TrigObjectMatcher.h:57:3: error: 'TrigObjectMatcher::TrigObjectMatcher(TrackSelector*, const std::vector<TrackTrigObject>&, bool (*)(const TrackTrigObject&, TIDA::Track*))' cannot be overloadedSo these certainly seem to be related to the changes in this MR.
-
TrigIDJpsiMonitoring is no longer being supported and should not be being compiled any longer. Charges were put in place to disable the compilation in the release, so presumably these have not been ported across.
TrogInDetAnalysisUser compiles ok in 21.3, I don't know why it would be failing in 21.9. I don't know the purpose of this branch, but we want no code development separately in this branch, we only will sweep from 21.3, so if it is fine in 21.3 then presumably it is due to something else in 21.9.
Mark
added review-user-action-required label and removed review-pending-level-1 label
-
Hi @sutt Sorry for the delayed reply - I was away last week.
TrogInDetAnalysisUser compiles ok in 21.3, I don't know why it would be failing in 21.9. I don't know the purpose of this branch, but we want no code development separately in this branch, we only will sweep from 21.3, so if it is fine in 21.3 then presumably it is due to something else in 21.9
21.9 is the branch for Phase 2 development, so it should be in principle 21.3 plus Phase 2 changes (mostly ITK) which should not really interfere with trigger stuff AFAIK.
We've merged quite a few trigger-related things recently so maybe now the issues have been resolved. I'll retry the CI to check.
-
This merge request affects 2 packages:
- Trigger/TrigAnalysis/TrigInDetAnalysis
- Trigger/TrigAnalysis/TrigInDetAnalysisUser
Adding @sutt as watcher
added review-pending-level-1 label and removed review-user-action-required label
-
Hi, many thanks, if it is just these TrigIDJpsiMonitoring issues, then we should see about simply removing that package from the branch. These was a MR a while back in master I think which removed them ...
so maybe we can back port this, although frankly with all these different branches it becomes very difficult to keeps things in line if there is different development in each branch.
Mark
-
Athena externals cmake make required tests optional tests Full details available at NICOS MR-20918-2019-04-29-18-19
Hi @sutt ,
does it mean you want this MR to stay open until TrigIDJpsiMonitoring issue is resolve?
L1 shifter
-
Hi, not sure what is meant by the question, I would prefer that this code was merged, but if the code is not compiling because of the TrigJpsiMonitoring, and the changes in master to exclude it can not be successfully swept to 21.9 then I guess we would have to close this and I would need to remove the TrigIDJpsiMonitoring code from 21.3 and then wait for that to be swept into 21.9.
I am somewhat puzzled however, that the code seems to be fine in 21.3, but not in 21.9, so perhaps 21.3 had some additional changes that failed to sweep into 21.9, so it might be a more fundamental underlying problem with the state of the code in 21.9.
My main concern would be that if I make a new change to 21.3 to remove the TrigIDJpsiMoniotring and this gets swept successfully, then the older changes from this sweep would then be out of date and would not be able to be swept, and if the code was in some sense already out of date then this might only make it worse.
I am not developing in this branch 21.9 so if the code can't be kept up to date with sweeps from 21.3, then I would have to simply ignore this branch.
Cheers Mark
-
I'm a bit confused here myself. I think what is happening is that if you look in TrigObjectMatcher.h in 21.9 it already has the function that this MR tries to add. Therefore, in effect once we add the changes here we just have two copies of exactly the same function declaration, which is why it complains about the overloading not being possible.
It really looks to me like we already have these changes in 21.9, so I think easiest is just to close this MR.
PS @swaban is no longer coordinating this release branch, it is now me and @asalzbur.
