Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

Comments

Close side panel

McParticle: moving default to 001#8048

Merged
jgrosseo merged 1 commit intoAliceO2Group:devAliceO2Group/AliceO2:devfrom
jgrosseo:mcjgrosseo/AliceO2:mcCopy head branch name to clipboard
Feb 1, 2022
Merged

McParticle: moving default to 001#8048
jgrosseo merged 1 commit intoAliceO2Group:devAliceO2Group/AliceO2:devfrom
jgrosseo:mcjgrosseo/AliceO2:mcCopy head branch name to clipboard

Conversation

@jgrosseo
Copy link
Collaborator

No description provided.

@jgrosseo
Copy link
Collaborator Author

@nburmaso I have removed the duplication of the table definition. Do I overlook something or is it not needed actually?
https://github.com/AliceO2Group/AliceO2/pull/8048/files#diff-d43bf1e210cd8966890634920e751cd6801d9125fe5c48e4268115b9dc211115L154

@jgrosseo
Copy link
Collaborator Author

This PR depends on AliceO2Group/O2Physics#488

@nburmaso
Copy link
Contributor

@nburmaso I have removed the duplication of the table definition. Do I overlook something or is it not needed actually? https://github.com/AliceO2Group/AliceO2/pull/8048/files#diff-d43bf1e210cd8966890634920e751cd6801d9125fe5c48e4268115b9dc211115L154

Hi @jgrosseo. I implemented a separate table for MC particles so that I had a table cursor without dynamic columns in it. At least it was needed at the moment when I first added MC particles to the converter. I guess MCParticlesTable can be removed now if it works fine with the StoredMcParticles_001

@jgrosseo jgrosseo marked this pull request as ready for review January 31, 2022 20:37
@jgrosseo jgrosseo requested review from a team as code owners January 31, 2022 20:37
@sawenzel
Copy link
Collaborator

sawenzel commented Feb 1, 2022

AliceO2Group/O2Physics#488 breaks the analysis testing which is done inside the full system test. This is unfortunately still the case with the PR here.

I see in the fullCI log:

++ grep 'Return status' sim-challenge.log
Return status of Efficiency: 1
Return status of EventTrackQA: 1

where the first seems to be a simple transition to the new table on the O2Physics side. The second one is a segfault.

@jgrosseo
Copy link
Collaborator Author

jgrosseo commented Feb 1, 2022

AliceO2Group/O2Physics#488 breaks the analysis testing which is done inside the full system test. This is unfortunately still the case with the PR here.

I see in the fullCI log:

++ grep 'Return status' sim-challenge.log
Return status of Efficiency: 1
Return status of EventTrackQA: 1

where the first seems to be a simple transition to the new table on the O2Physics side. The second one is a segfault.

Do you know what needs to be fixed?

Copy link
Collaborator

@sawenzel sawenzel left a comment

Choose a reason for hiding this comment

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

Ok let's merge it. FullCI is back to normal in a local test.

@sawenzel
Copy link
Collaborator

sawenzel commented Feb 1, 2022

Remaining issues followed up here: https://alice.its.cern.ch/jira/browse/O2-2766

@jgrosseo jgrosseo merged commit adce1ce into AliceO2Group:dev Feb 1, 2022
@jgrosseo jgrosseo deleted the mc branch February 1, 2022 09:42
@jgrosseo
Copy link
Collaborator Author

jgrosseo commented Feb 1, 2022

Thanks a lot!

@jgrosseo
Copy link
Collaborator Author

jgrosseo commented Feb 1, 2022

I have asked @ktf for a daily tag. Then we can merge AliceO2Group/O2Physics#491 and all tests should be back in business.

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.

3 participants

Morty Proxy This is a proxified and sanitized view of the page, visit original site.