Skip to content

Conversation

@theHenks
Copy link

@theHenks theHenks commented Jan 20, 2026

Parse o/o and o/oo as percent and permille units while loading. These units are used in the context of the FlashCam decoding in the extra section (s. here)

Why need ASAP a description of units in the dataformat-specs. This is way too much work to debug every time.......

c.f. @gipert @ggmarshall

@DaGeibl : I still need to update the tests/ on LegendHDF5IO.jl and the bump the compat for this too work properly.

@theHenks theHenks requested a review from oschulz January 20, 2026 16:54
@theHenks theHenks self-assigned this Jan 20, 2026
@theHenks theHenks added the bug Something isn't working label Jan 20, 2026
@gipert
Copy link
Member

gipert commented Jan 20, 2026

well these are also not physical units... otherwise we also accept ADC

@theHenks
Copy link
Author

well these are also not physical units... otherwise we also accept ADC

However, we can parse them into Unitful The unit percent and permille exist. The question is if we wanna drop them in the future or not.

@oschulz
Copy link
Contributor

oschulz commented Jan 20, 2026

I see little value in keeping such units around, I would definitely prefer stripping them. What would be the use case for such "units"?

@theHenks
Copy link
Author

I see little value in keeping such units around, I would definitely prefer stripping them. What would be the use case for such "units"?

We can also strip them. However, you loose now information that the numbers are permille. At least in the context of JuLeAna, we save so far percent etc as units, e.g., in the pars output for survival fractions etc.

@oschulz
Copy link
Contributor

oschulz commented Jan 20, 2026

We can also strip them. However, you loose now information that the numbers are permille

Ah - no, with stripping I mean converting them to a normal fraction value.

@oschulz
Copy link
Contributor

oschulz commented Jan 20, 2026

well these are also not physical units... otherwise we also accept ADC

It's not quite the same - ADC is not a universally comparable thing, percent is well-define. Technically, it's not a unit, but a mathematical constant, but for practical purposes one can treat it as kind of a unit.

@oschulz
Copy link
Contributor

oschulz commented Jan 20, 2026

But why o/o and not % and o/oo and not ?

@theHenks
Copy link
Author

But why o/o and not % and o/oo and not ?

I don't know. But this can be changed in the link I send. I think this all comes down again to a dataformat spec concerning units...

@theHenks
Copy link
Author

well these are also not physical units... otherwise we also accept ADC

It's not quite the same - ADC is not a universally comparable thing, percent is well-define. Technically, it's not a unit, but a mathematical constant, but for practical purposes one can treat it as kind of a unit.

As I said, Unitful is treating it as a unit.

@oschulz
Copy link
Contributor

oschulz commented Jan 20, 2026

I don't know. But this can be changed in the link I send. I think this all comes down again to a dataformat spec concerning units

Is this mainly intended for output that's both human and machines-readable, though? Do we already use it somewhere except for survival fractions in pars?

@theHenks
Copy link
Author

I don't know. But this can be changed in the link I send. I think this all comes down again to a dataformat spec concerning units

Is this mainly intended for output that's both human and machines-readable, though? Do we already use it somewhere except for survival fractions in pars?

Dont know, maybe @gipert can answer.

@gipert
Copy link
Member

gipert commented Jan 21, 2026

I think it's pretty useless to store values in percent or permill...

what punishment should we inflict to @ssailer?

@ssailer
Copy link

ssailer commented Jan 21, 2026

Please discuss this with the Physics King. I was told, we're supposed to make identical copies of the daq files.

@theHenks
Copy link
Author

Please discuss this with the Physics King. I was told, we're supposed to make identical copies of the daq files.

Is that @ggmarshall ? I guess whatever we do we probably won't reprocess all daq files again to change the unit in these couple keys, so we have to fix it here independent of that.
My proposal:

  • Let's drop the units here and get rid of them followed by updating LegendHDF5IO (and the tests) and check the testdata (someone from pygama team)
  • PR to legend-dataformat-specs with a unit definition. All units according to the SI unit conventions, no percent, permille, ADC etc.

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

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants