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

CPV raw writing and reconstruction#5129

Merged
shahor02 merged 10 commits intoAliceO2Group:devAliceO2Group/AliceO2:devfrom
peressounko:devperessounko/AliceO2:devCopy head branch name to clipboard
Jan 11, 2021
Merged

CPV raw writing and reconstruction#5129
shahor02 merged 10 commits intoAliceO2Group:devAliceO2Group/AliceO2:devfrom
peressounko:devperessounko/AliceO2:devCopy head branch name to clipboard

Conversation

@peressounko
Copy link
Collaborator

@peressounko peressounko commented Jan 6, 2021

Code to write and read CPV raw data, convert raw->digits->clusters->CTF

@peressounko peressounko requested review from a team, kharlov, sawenzel and shahor02 as code owners January 6, 2021 15:38
@davidrohr
Copy link
Collaborator

davidrohr commented Jan 7, 2021

@ktf @TimoWilken @pzhristov @costing : During O2 compilation, we are getting warnings from ROOT about JAlien:

Error in <TSystem::ExpandFileName>: input: $ROOT_DYN_PATH/libRIO, output: $ROOT_DYN_PATH/libRIO
Error in <TSystem::ExpandFileName>: input: $ROOT_DYN_PATH/libCling, output: $ROOT_DYN_PATH/libCling
Error in cling::AutoloadingVisitor::InsertIntoAutoloadingState:
   Missing FileEntry for TJAlienCollection.h
   requested to autoload type TAliceCollection
Error in cling::AutoloadingVisitor::InsertIntoAutoloadingState:
   Missing FileEntry for TJAlienCollection.h
   requested to autoload type TJAlienCollection
Error in cling::AutoloadingVisitor::InsertIntoAutoloadingState:
   Missing FileEntry for TJAlienCredentials.h
   requested to autoload type TJAlienCredentials
Error in cling::AutoloadingVisitor::InsertIntoAutoloadingState:
   Missing FileEntry for TJAlien.h
   requested to autoload type TJAlienJDL
Error in cling::AutoloadingVisitor::InsertIntoAutoloadingState:
   Missing FileEntry for TJAlien.h
   requested to autoload type TAliceFile
Error in cling::AutoloadingVisitor::InsertIntoAutoloadingState:
   Missing FileEntry for TJAlien.h
   requested to autoload type TJAlienFile

Could it be that some JAlien dictionary is missing? This does not break the build, since we only fail on compiler warnings and this comes from ROOT. But it fills the logs, and I think will be confusing for people when a PR fails and they look for the real compile errors. Any ideas?

@shahor02
Copy link
Collaborator

shahor02 commented Jan 8, 2021

@peressounko Please fix what CI complains for:

/mnt/mesos/sandbox/sandbox/o2-fullci/sw/SOURCES/O2/5129/0/Detectors/CPV/reconstruction/include/CPVReconstruction/RawDecoder.h:31:3: error: use '= default' to define a trivial copy constructor [modernize-use-equals-default]
/mnt/mesos/sandbox/sandbox/o2-fullci/sw/SOURCES/O2/5129/0/Detectors/CPV/reconstruction/src/RawDecoder.cxx:148:29: error: statement should be inside braces [readability-braces-around-statements]
/mnt/mesos/sandbox/sandbox/o2-fullci/sw/SOURCES/O2/5129/0/Detectors/PHOS/workflow/src/RawToCellConverterSpec.cxx:138:23: error: statement should be inside braces [readability-braces-around-statements]
/mnt/mesos/sandbox/sandbox/o2-fullci/sw/SOURCES/O2/5129/0/Detectors/CPV/reconstruction/include/CPVReconstruction/RawDecoder.h:31:3: error: use '= default' to define a trivial copy constructor [modernize-use-equals-default]
/mnt/mesos/sandbox/sandbox/o2-fullci/sw/SOURCES/O2/5129/0/Detectors/CPV/reconstruction/include/CPVReconstruction/RawDecoder.h:31:3: error: use '= default' to define a trivial copy constructor [modernize-use-equals-default]

shahor02
shahor02 requested changes Jan 8, 2021
View reviewed changes
Copy link
Collaborator

@shahor02 shahor02 left a comment

Choose a reason for hiding this comment

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

@peressounko
Thanks! Could you also add test_ctf_io_cpv.cxx similar to test_ctf_io_phos.cxx?

DataFormats/Detectors/CPV/include/DataFormatsCPV/Cluster.h Outdated
char mMulDigit = 0; ///< Digit nultiplicity
char mModule = 0; ///< Module number
char mNExMax = -1; ///< number of (Ex-)maxima before unfolding
int mLabel = -1; ///< Ref to entry in MCTruthContainer with list of labels
Copy link
Collaborator

@shahor02 shahor02 Jan 8, 2021

Choose a reason for hiding this comment

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

I see you are setting this label but don't see where it is used. The same for PHOS clusters. Do you need them? Usually we extract MC labels from the container using the cluster index in its container.

Copy link
Collaborator Author

@peressounko peressounko Jan 8, 2021

Choose a reason for hiding this comment

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

Thanks, this is rudiment, removed.

@peressounko peressounko requested a review from a team as a code owner January 8, 2021 22:21
@shahor02
Copy link
Collaborator

shahor02 commented Jan 10, 2021

@peressounko the CTF/test/test_ctf_io_cpv.cxx fails. You can test it locally by e.g.

cd alice/sw/BUILD/O2-latest/O2/
./stage/tests/o2-test-ctf-cpv

@peressounko
Copy link
Collaborator Author

peressounko commented Jan 11, 2021

Dear @shahor02, could you please approve this PR to make test of CPV code easier to cpv experts? Thanks, Dmitri

@ktf
Copy link
Member

ktf commented Jan 11, 2021

Please fix the fullCI issue.

@shahor02
Copy link
Collaborator

shahor02 commented Jan 11, 2021

Hi @peressounko , sure, will merge once the tests are passed.

@shahor02 shahor02 merged commit 47bb2dd into AliceO2Group:dev Jan 11, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@shahor02 shahor02 shahor02 approved these changes

@kharlov kharlov Awaiting requested review from kharlov kharlov is a code owner

@sawenzel sawenzel Awaiting requested review from sawenzel sawenzel is a code owner

Assignees

No one assigned

Labels

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@peressounko @davidrohr @shahor02 @ktf
Morty Proxy This is a proxified and sanitized view of the page, visit original site.