Skip to content

TOF: Add unfied container for parameters on ccdb - #10364

Merged
shahor02 merged 2 commits into
AliceO2Group:devfrom
njacazio:nj-ccdb-paramstorage
Nov 29, 2022
Merged

TOF: Add unfied container for parameters on ccdb#10364
shahor02 merged 2 commits into
AliceO2Group:devfrom
njacazio:nj-ccdb-paramstorage

Conversation

@njacazio

@njacazionjacazio commented Nov 25, 2022

Copy link
Copy Markdown
Collaborator

@noferini

  • add container to store pass and parameters in a map scheme

@njacazio
njacazioforce-pushed the nj-ccdb-paramstorage branch 2 times, most recently from 47f75b8 to eba169fCompareNovember 25, 2022 22:06
- add container to store pass and parameters in a map scheme
Co-authored-by: noferini <noferini@bo.infn.it>
@njacazio

Copy link
Copy Markdown
CollaboratorAuthor

@noferini I had to rework a bit the code and the naming of the member functions could you confirm that it works with your macro?

@njacazio
njacazio marked this pull request as ready for review November 26, 2022 13:24
@noferini

Copy link
Copy Markdown
Collaborator

@noferini I had to rework a bit the code and the naming of the member functions could you confirm that it works with your macro?

I sent you the macro

noferini
noferini previously approved these changes Nov 26, 2022

@shahor02shahor02 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

The error

 LOG(debug) << "Changing parametrization corresponding to key " << key << " from size " << mParameters[key].size() < " to " << p.GetName() << " of size " << p.size();
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ ^ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

is genuine.

@njacazio

Copy link
Copy Markdown
CollaboratorAuthor

Hi @shahor02 indeed, thanks for spotting this

@shahor02shahor02 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

@njacazio rechecking the code: don't you need to add a dictionary (ClassDefN and entry in the LinkDef.h) for the Parameters class?
Note that for the templated classes the template parameter change is not supported by the root schema evolution, i.e. a legacy Parameter<A> will not readable with Parameter<B> even with class version change.

@njacazio

Copy link
Copy Markdown
CollaboratorAuthor

@shahor02 thanks a lot for the comment!
Indeed I think it will be fine, in principle we don't need the schema evolution. The Parameter<int> class is just a way to define sets of parameters with defined names (e.g. in https://github.com/AliceO2Group/AliceO2/pull/10364/files#diff-ca1417bbfb70e51406dde90ebbaf76c473f283d1c82548356e2093828d9228a4R64) that will make it easy to distribute the parameters taken from the ParameterCollection expected to be stored on the ccdb.
I hope I understood your point correctly and this clears your doubts.

@shahor02

Copy link
Copy Markdown
Collaborator

@njacazio OK for template, but since this is persistent object, I still think you need a ClassDefNF + entry in the LinkDef.

@noferini

Copy link
Copy Markdown
Collaborator

Hi, shall we merge this PR?

@shahor02

Copy link
Copy Markdown
Collaborator

Hi,
I can merge it, but did you check that you can write and read back the object? As I wrote above, the Parameters class, which is supposed to be persistent, has not ClassDefNV

@noferini

Copy link
Copy Markdown
Collaborator

Hi @shahor02,
I didn't have any problem to write and read.

TFile algFile("ccdb.root", "recreate");
algFile.WriteObjectAny(&tof, "o2::tof::ParameterCollection", "ccdb_object");
algFile.Close();

TFile *f = new TFile("ccdb.root");
o2::tof::ParameterCollection *a = (o2::tof::ParameterCollection *) f->Get("ccdb_object");

@shahor02

Copy link
Copy Markdown
Collaborator

OK, thanks, apparently it is able to use the ClassDef of the derived class. Merging

@shahor02
shahor02 merged commit cbedd7f into AliceO2Group:devNov 29, 2022
@njacazio
njacazio deleted the nj-ccdb-paramstorage branch November 29, 2022 15:05
iouribelikov pushed a commit to iouribelikov/AliceO2 that referenced this pull request Dec 16, 2022
* TOF: Add unfied container for parameters on ccdb
- add container to store pass and parameters in a map scheme
Co-authored-by: noferini <noferini@bo.infn.it>
* Update ParameterContainers.cxx
Co-authored-by: noferini <noferini@bo.infn.it>
Sign up for freeto 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

@njacazio@noferini@shahor02