Skip to content

Add Converter for new track extra dataformat #3572

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
ddobrigk merged 4 commits into from
Oct 21, 2023
Merged

Add Converter for new track extra dataformat #3572

ddobrigk merged 4 commits into from
Oct 21, 2023

Conversation

@fmazzasc
Copy link
Collaborator

@fmazzasc fmazzasc commented Oct 9, 2023

Pinging @mpuccio , @ddobrigk
This has to be merged after #12044(O2), could you have a look at this in the meanwhile?

@fmazzasc fmazzasc marked this pull request as draft October 9, 2023 10:38
@fmazzasc fmazzasc mentioned this pull request Oct 16, 2023
Merged
@fmazzasc fmazzasc marked this pull request as ready for review October 19, 2023 08:50
@BongHwi BongHwi enabled auto-merge (squash) October 19, 2023 08:50
@BongHwi BongHwi disabled auto-merge October 19, 2023 08:51
@fmazzasc
Copy link
Collaborator Author

fmazzasc commented Oct 19, 2023

Pinging also @jgrosseo

@alibuild
Copy link
Collaborator

alibuild commented Oct 19, 2023

Error while checking build/O2Physics/o2 for c0d8461 at 2023-10-19 11:23:

## sw/BUILD/O2Physics-latest/log
/sw/SOURCES/O2Physics/3572-slc7_x86-64/0/Common/TableProducer/tracksextraConverter.cxx:20:17: error: 'StoredTracksExtra_001' is not a member of 'o2::aod'; did you mean 'StoredTracksExtra'?
/sw/SOURCES/O2Physics/3572-slc7_x86-64/0/Common/TableProducer/tracksextraConverter.cxx:20:38: error: template argument 1 is invalid
/sw/SOURCES/O2Physics/3572-slc7_x86-64/0/Common/TableProducer/tracksextraConverter.cxx:21:16: error: 'o2::aod::TracksExtra_000' has not been declared
/sw/SOURCES/O2Physics/3572-slc7_x86-64/0/Common/TableProducer/tracksextraConverter.cxx:26:25: error: 'begin' was not declared in this scope
/sw/SOURCES/O2Physics/3572-slc7_x86-64/0/Common/TableProducer/tracksextraConverter.cxx:26:25: error: 'end' was not declared in this scope
/sw/SOURCES/O2Physics/3572-slc7_x86-64/0/Common/TableProducer/tracksextraConverter.cxx:54:44: error: expression cannot be used as a function
/sw/SOURCES/O2Physics/3572-slc7_x86-64/0/Common/TableProducer/tracksextraConverter.cxx:61:15: error: 'TracksExtra_001' is not a member of 'o2::aod'; did you mean 'TracksExtra'?
/sw/SOURCES/O2Physics/3572-slc7_x86-64/0/Common/TableProducer/tracksextraConverter.cxx:61:30: error: template argument 1 is invalid
/sw/slc7_x86-64/O2/daily-20231019-0200-local1/include/Framework/ASoA.h:148:26: error: 'decayed' {aka 'int'} is not a class, struct, or union type
/sw/slc7_x86-64/O2/daily-20231019-0200-local1/include/Framework/ASoA.h:149:51: error: 'decayed' {aka 'int'} is not a class, struct, or union type
/sw/slc7_x86-64/O2/daily-20231019-0200-local1/include/Framework/AnalysisTask.h:240:55: error: too many braces around scalar initializer for type 'int'
/sw/slc7_x86-64/O2/daily-20231019-0200-local1/include/Framework/AnalysisTask.h:341:51: error: static assertion failed: Single argument of process() should be a table-like or an iterator
/sw/slc7_x86-64/O2/daily-20231019-0200-local1/include/Framework/AnalysisTask.h:117:50: error: static assertion failed: Could not find metadata. Did you register your type?
/sw/slc7_x86-64/O2/daily-20231019-0200-local1/include/Framework/AnalysisTask.h:125:51: error: incomplete type 'metadata' {aka 'void'} used in nested name specifier
/sw/slc7_x86-64/O2/daily-20231019-0200-local1/include/Framework/AnalysisTask.h:125:71: error: incomplete type 'metadata' {aka 'void'} used in nested name specifier
/sw/slc7_x86-64/O2/daily-20231019-0200-local1/include/Framework/AnalysisTask.h:125:96: error: incomplete type 'metadata' {aka 'void'} used in nested name specifier
/sw/slc7_x86-64/O2/daily-20231019-0200-local1/include/Framework/AnalysisTask.h:125:117: error: incomplete type 'metadata' {aka 'void'} used in nested name specifier
/sw/slc7_x86-64/O2/daily-20231019-0200-local1/include/Framework/AnalysisTask.h:125:117: error: no matching function for call to 'o2::framework::InputSpec::InputSpec(<brace-enclosed initializer list>)'
ninja: build stopped: subcommand failed.

Full log here.

@jgrosseo
Copy link
Contributor

jgrosseo commented Oct 19, 2023

The CI will fail until the next O2 tag.

jgrosseo
jgrosseo reviewed Oct 19, 2023
View reviewed changes
Common/TableProducer/tracksextraConverter.cxx
void process(aod::TracksExtra_000 const& tracksExtra_000)
{

// dummy itsClusterSizes, fill with overflows if a hit in the layer is present
Copy link
Contributor

@jgrosseo jgrosseo Oct 19, 2023

Choose a reason for hiding this comment

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

Is overflow really the best choice? Not just 1 maybe?

Copy link
Collaborator Author

@fmazzasc fmazzasc Oct 19, 2023

Choose a reason for hiding this comment

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

I think it would be more difficult to distinguish whether this is a dummy entry or not as it is much more probable to have 1-size clusters than > 14-size clusters.

@fmazzasc
Copy link
Collaborator Author

fmazzasc commented Oct 21, 2023

CI Tests are passing now. Could we merge this? @jgrosseo @ddobrigk

ddobrigk
ddobrigk approved these changes Oct 21, 2023
View reviewed changes
Copy link
Collaborator

@ddobrigk ddobrigk left a comment

Choose a reason for hiding this comment

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

Thanks @fmazzasc, looks good to me!

@ddobrigk ddobrigk merged commit fd4b69c into AliceO2Group:master Oct 21, 2023
vkucera
vkucera reviewed Oct 22, 2023
View reviewed changes
Copy link
Collaborator

@vkucera vkucera left a comment

Choose a reason for hiding this comment

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

Sorry for commenting after merging, but there are several issues to fix.

  • The names of the executable, file and tasks do not follow the naming conventions.
    • executable: tracksextra-converter -> tracks-extra-converter
    • file: tracksextraConverter.cxx -> tracksExtraConverter.cxx
    • tasks: tracksextraConverter -> TracksExtraConverter, trackExtraSpawner -> TracksExtraSpawner
  • Mandatory Doxygen documentation is missing.
  • The track loop is missing const.

zconesa pushed a commit to zconesa/O2Physics that referenced this pull request Oct 27, 2023
@fmazzasc @zconesa
Add Converter for new track extra dataformat (AliceO2Group#3572)
1c62b8a 
* Add track extra converter

* Update converter and remove cluster map from trackExtra_001

* Spawn the extended table in the converter

* Restore ontheflytracker
chengtt0406 pushed a commit to chengtt0406/O2Physics that referenced this pull request Dec 6, 2023
@fmazzasc @chengtt0406
Add Converter for new track extra dataformat (AliceO2Group#3572)
4aa6839 
* Add track extra converter

* Update converter and remove cluster map from trackExtra_001

* Spawn the extended table in the converter

* Restore ontheflytracker
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jgrosseo jgrosseo jgrosseo left review comments

@vkucera vkucera vkucera left review comments

@BongHwi BongHwi BongHwi approved these changes

@ddobrigk ddobrigk ddobrigk approved these changes

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

@njacazio njacazio Awaiting requested review from njacazio

@hscheid hscheid Awaiting requested review from hscheid

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

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

Assignees

No one assigned

Labels

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@fmazzasc @alibuild @jgrosseo @BongHwi @vkucera @ddobrigk