Draw short integers as numbers, not characters

[swertz]

@Wile_E_Coyote

Always make a “counter” a “signed integer”

Strange, we have had it as unsigned (with /i) in NanoAOD for years and it’s never caused any issue… In any case, that doesn’t solve the above problem.

@Wile_E_Coyote

BTW. Why don’t you simply try (no need for the “nJet” at all):
auto br = t.Branch("Jet_idx", &temp);

Jet_idx is a variable-size array, how would that work?

[Wile_E_Coyote]

@swertz

we have had it as unsigned (with /i) in NanoAOD for years

Yes, I know this problem since years ago here.
It’s time you fix it.
The current statement is clear: The leaf referred to by nelem **MUST** be an int (/I)
Or you ask @pcanal and/or @Axel to finally support “unsigned integers” as [nelem] variable-size lengths.

@swertz

Jet_idx is a variable-size array, how would that work?

A “std::vector” is by definition “variable-size”, too.

[swertz]

@Wile_E_Coyote

Yes, I know this problem since years ago here.
It’s time you fix it.

I’m only now learning about this… I’ll put it on the my list. Why is it such a problem if it’s been working fine for years?

@Wile_E_Coyote

A “std::vector” is by definition “variable-size”, too.

Ah yes, OK. That means changing from arrays to STL vectors, this is really a change in the NanoAOD format which would potentially break many things downstream, so it’s not something we would do lightly.

[Wile_E_Coyote]

@swertz

it’s been working fine for years

Sometimes some things worked, and some did not.
You’d have to go through various posts here.

@swertz

That means changing from arrays to STL vectors

If you want to stay with the “arrays”, as provided by ROOT, you are limited to its predefined variables’ “fundamental types”.

[swertz]

@Wile_E_Coyote

Sometimes some things worked, and some did not.
You’d have to go through various posts here.

If these problems have never been reported to the NanoAOD developers, we cannot be expected to know about them.

@Wile_E_Coyote

If you want to stay with the “arrays”, as provided by ROOT, you are limited to its predefined variables’ “fundamental types”.

It’s what I’m doing. I want to store 8 bit signed integer, and have them treated like numbers, not characters.

[Wile_E_Coyote]

@swertz

I want to store 8 bit signed integer

Draw short integers as numbers, not characters

TTree does NOT know “int8_t” or “uint8_t” as variables’ “fundamental types”. It only knows “Char_t” (“signed char”) and “UChar_t” (unsigned char), where “char” is assumed to be 8-bits wide. https://root-forum.cern.ch/t/draw-short-integers-as-numbers-not-characters/52530/15 BTW. I think, ROOT unconditionally expects that a “char” is a “signed char”.

[swertz]

Thanks for your useful help and the cordial discussion.

[Wile_E_Coyote]

@couet Maybe one should fix this description? Instead of the word “integer”, there should be the word “character”.
Also, maybe the explicit statement “The leaf referred to by nelem **MUST** be an int (/I)” should be copied from the hidden TBranch::TBranch description to the TTree → Add a column (“branch”) holding fundamental types and arrays thereof

@pcanal and/or @Axel … is there any chance that other types of variables will be supported as [nelem] variable-size lengths (at least “unsigned int”, but maybe also other integer types would be useful in a long run).
Same for the TTreeReader, of course:

TTreeReaderArray with branch of variable size array

I think I understand (@eguiraud - the TTreeReaderArray currently sets up a TTreeReaderValue to read the branch / leaf count, and it got its underlying type wrong): In principle, only int is supported as array size. But in reality we never really enforced that, so now TTreeReader needs to handle this case. I’m (I was, actually) working on changing the way TTreeReaderArray reads its data; let me resume that - soon - and I’ll check this case. I have created https://sft.its.cern.ch/jira/browse/ROO…

[couet]

@Wile_E_Coyote

@couet Maybe one should fix this description? Instead of the word “integer”, there should be the word “character”.
Also, maybe the explicit statement “The leaf referred to by nelem **MUST** be an int (/I)” should be copied from the hidden TBranch::TBranch description to the TTree → Add a column (“branch”) holding fundamental types and arrays thereof

As it is in the TTree documentation I let @pcanal comment.

[pcanal]

@swertz

Why is it such a problem if it’s been working fine for years?

Internally, TTree will use the counter as a signed integer independently of what you specified. This could be a problem for very large value of the counter. In particular terms as mentioned by Wile, there are some code path and code pattern that reject or misuse the unsigned integer and some that don’t. So even-though it seems to work in most case, it is better to avoid the mismatch (and unfortunately updating to properly support other type is non-trivial).

Why can’t int8_t be interpreted as a number by default?

Because for a long time, this was the only way to store textual labels (and the TBrowser can not tell what is the semantic of the fields, i.e. label or value ).

[pcanal]

@couet

As it is in the TTree documentation I let @pcanal comment.

Wile’s proposals sounds good. @couet Could you implement them?

[pcanal]

@swertz

auto br = t.Branch("Jet_idx", &(temp.front()), "Jet_idx[nJet]");

This, implicitly, request the branch to be create to hold floats. Since the 2 have the same size, it might still appear to work but the automatic tool will definitively get it wrong (Draw, Browser, etc.)

[pcanal]

However I don’t want people using those files to have to do tricks like Jet_idx+0 when drawing branches…

You have 2 potential options. Either use an unsigned char or a signed short (yes, at double the storage cost but automatic tools will work properly and some of the extra storage space will be recovered by compression).

[swertz]

Thanks for the clarifications @pcanal ! We will switch to signed integers for the counters in nanoAOD. Using 16 bit integers instead of 8 bit could be an intereting middle ground indeed, I’ll give it a try.

[couet]

@pcanal

Wile’s proposals sounds good. @couet Could you implement them?

@pcanal: The PR is here. Let me know if it’s fine for you.

[Wile_E_Coyote]

@couet I think:

The `myvar` column, which holds the variable size, **MUST** be an `Int_t` (/I).

message should also explicitly appear immediately in the TTree → “fundamental types” section, at the end of the “Examples:” (it’s called there “myvar” instead of “nelem”), as almost nobody will ever read the TTree::Branch method description (well, even I forgot that it needed fixes, too).

And maybe modify also:

... and the size of which is stored in an `Int_t` (/I) branch called `myArrSize`, ...`

[couet]

@Wile_E_Coyote

message should also explicitly appear immediately in the TTree → “fundamental types” section, at the end of the “Examples

done. see: [skip-ci] Update ttree doc by couet · Pull Request #11847 · root-project/root · GitHub

@Wile_E_Coyote

And maybe modify also:

... and the size of which is stored in an `Int_t` (/I) branch called `myArrSize`, ...`

Not sure what you mean by that …

[Wile_E_Coyote]

@couet Just see the last point in the TTree → “fundamental types” section … I propose to modify it a bit (instead of “in a branch”, explicitly say “in an Int_t (/I) branch”).

[couet]

@Wile_E_Coyote . Oh I see. Thanks ! Update done. See:

[skip-ci] Update ttree doc