Skip to content

Conversation

@fmazzasc
Copy link
Contributor

@fmazzasc fmazzasc marked this pull request as draft October 16, 2023 12:36
@fmazzasc fmazzasc marked this pull request as ready for review October 22, 2023 10:45
@fmazzasc
Copy link
Contributor Author

@jgrosseo , @aalkin , @mpuccio

I had to define the old implementation of ITSNCls , ITSNClsInnerBarrel , ITSClusterMap outside the namespace v000 . The reason is that there are many O2Physics derived data models relying on these columns and I did not want to modify all of them. See for example:

If you don't like this change I can put all the columns under the namespace, then change in O2Physics all the columns relying on track::ITSClusterMap or track::ITSNCls into track::v000::ITSClusterMap and track::v000::ITSNCls . If we choose this option, we have to be aware that the compilation of O2Physics would fail between the merging of this P.R. and the patch in O2Physics.
What do you think? Any smarter ideas?

@jgrosseo
Copy link
Collaborator

Frankly, I would leave the old columns in the tracks namespace and the new ones in tracks::v001 (as already merged). I think the rest is confusing.

@fmazzasc
Copy link
Contributor Author

@jgrosseo , I agree with you. v001 restored

@jgrosseo
Copy link
Collaborator

@jgrosseo , I agree with you. v001 restored

Thanks!
Still the diff moves some code around. Could you clean this up? We should make the minimal changes needed...

@jgrosseo
Copy link
Collaborator

And isn't the change of filling the table in the producer missing?

@fmazzasc
Copy link
Contributor Author

@jgrosseo , minimal changes done. And the filling of the AODProducer should be more visible now ;)

@jgrosseo
Copy link
Collaborator

Looks good!

Before merging, please test if the converter works on Hyperloop. Once you have successfully tested, I can clone it as service wagon

@fmazzasc
Copy link
Contributor Author

I tested it on hyperloop and it works. But I would still wait for AliceO2Group/O2Physics#3671 to be merged, as the converter name will change

@alibuild
Copy link
Collaborator

alibuild commented Oct 26, 2023

Error while checking build/O2/fullCI for 06e6ec3 at 2023-10-26 09:46:

## sw/BUILD/O2-latest/log
c++: error: unrecognized command-line option '--rtlib=compiler-rt'
c++: error: unrecognized command-line option '--rtlib=compiler-rt'
c++: error: unrecognized command-line option '--rtlib=compiler-rt'
c++: error: unrecognized command-line option '--rtlib=compiler-rt'


## sw/BUILD/QualityControl-latest/log
c++: error: unrecognized command-line option '--rtlib=compiler-rt'
c++: error: unrecognized command-line option '--rtlib=compiler-rt'
c++: error: unrecognized command-line option '--rtlib=compiler-rt'
c++: error: unrecognized command-line option '--rtlib=compiler-rt'


## sw/BUILD/O2Physics-latest/log
c++: error: unrecognized command-line option '--rtlib=compiler-rt'
c++: error: unrecognized command-line option '--rtlib=compiler-rt'
c++: error: unrecognized command-line option '--rtlib=compiler-rt'
c++: error: unrecognized command-line option '--rtlib=compiler-rt'
[ERROR] Function RecoDecay::getMassPDG is deprecated and will be removed soon.
[ERROR] Please use the Mass function in the O2DatabasePDG service instead.
[ERROR] See the example of usage in Tutorials/src/usingPDGService.cxx.
[ERROR] Function RecoDecay::getMassPDG is deprecated and will be removed soon.
[ERROR] Please use the Mass function in the O2DatabasePDG service instead.
[ERROR] See the example of usage in Tutorials/src/usingPDGService.cxx.
[ERROR] Function RecoDecay::getMassPDG is deprecated and will be removed soon.
[ERROR] Please use the Mass function in the O2DatabasePDG service instead.
[ERROR] See the example of usage in Tutorials/src/usingPDGService.cxx.
[ERROR] Function RecoDecay::getMassPDG is deprecated and will be removed soon.
[ERROR] Please use the Mass function in the O2DatabasePDG service instead.
[ERROR] See the example of usage in Tutorials/src/usingPDGService.cxx.


## sw/BUILD/O2-full-system-test-latest/log
Detected critical problem in logfile reco_NOGPU.log
reco_NOGPU.log:[51777:calib-emcalchannel-badchannel]: [07:46:13][FATAL] error while setting up workflow in o2-calibration-emcal-channel-calib-workflow: filesystem error: cannot create directories: Permission denied [/scratch/services/detector_tmp/emc_calib]
[51773:BadMapCalibSpec]: [07:45:59][ERROR] Insufficient statistics: 11 entries in lowE histo, do nothing
[51777:calib-emcalchannel-badchannel]: [07:46:13][FATAL] error while setting up workflow in o2-calibration-emcal-channel-calib-workflow: filesystem error: cannot create directories: Permission denied [/scratch/services/detector_tmp/emc_calib]
[ERROR] pid 51777 (calib-emcalchannel-badchannel) crashed with or was killed with exit code 1
[ERROR]  - Device calib-emcalchannel-badchannel: pid 51777 (exit 1)
[INFO]    - First error: [07:46:13][FATAL] error while setting up workflow in o2-calibration-emcal-channel-calib-workflow: filesystem error: cannot create directories: Permission denied [/scratch/services/detector_tmp/emc_calib]
[ERROR] SEVERE: Device calib-emcalchannel-badchannel (51777) had at least one message above severity 5: error while setting up workflow in o2-calibration-emcal-channel-calib-workflow: filesystem error: cannot create directories: Permission denied [/scratch/services/detector_tmp/emc_calib]


## sw/BUILD/o2checkcode-latest/log
--
========== List of errors found ==========
++ GRERR=0
++ grep -v clang-diagnostic-error error-log.txt
[0 more errors; see full log]

Full log here.

@fmazzasc
Copy link
Contributor Author

Hi @jgrosseo , @ddobrigk , the tracks-extra-converter is now ready and has the correct name. I tested it on hyperloop and I cross checked again that it preserves the correct ITSClusterMap. Whenever you want, you can merge this P.R. and create the service wagon

@jgrosseo jgrosseo enabled auto-merge (squash) October 26, 2023 12:53
@jgrosseo
Copy link
Collaborator

I have put auto merge and then wait for fullCI
@ddobrigk Could you create the service wagon and alert people? Myself I will be offline this afternoon...

@ddobrigk
Copy link
Contributor

@fmazzasc @jgrosseo thanks a lot! Service wagon has been created here: https://alimonitor.cern.ch/hyperloop/view-wagon/6495 - I will write an announcement message when merged.

@fmazzasc fmazzasc requested a review from jgrosseo October 26, 2023 13:15
@ddobrigk
Copy link
Contributor

Hmmm not sure why this isn't merged, I guess we still miss an approval by someone?... (I have no permissions)

@mconcas mconcas disabled auto-merge October 27, 2023 09:21
@mconcas mconcas merged commit 7774a9e into AliceO2Group:dev Oct 27, 2023
christianreckziegel pushed a commit to LucasFerrandi/AliceO2 that referenced this pull request Nov 9, 2023
* Set TrackExtra001 as default

* Fix test

* Restore v001

* Minimal change of AnalysisDataModel
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.

5 participants