refactor: Consistently use mm for time in examples IO

[andiwand]

We are mixing time units in examples currently. I propose to solve this by consistently using mm which is the native internal core unit

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (d5f264e) 48.84% compared to head (d4a1c79) 48.84%.

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #2916   +/-   ##
=======================================
  Coverage   48.84%   48.84%           
=======================================
  Files         495      495           
  Lines       28909    28909           
  Branches    13732    13732           
=======================================
  Hits        14122    14122           
  Misses       4890     4890           
  Partials     9897     9897           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[github-actions]

📊: Physics performance monitoring for d4a1c79

Full contents

physmon summary

• ✅ CKF truth_smeared
• ✅ IVF truth_smeared
• ✅ AMVF truth_smeared
• ✅ Track Summary CKF truth_smeared
• ✅ Seeding truth_estimated
• ✅ CKF truth_estimated
• ✅ IVF truth_estimated
• ✅ AMVF truth_estimated
• ✅ Track Summary CKF truth_estimated
• ✅ Seeding seeded
• ✅ CKF seeded
• ✅ IVF seeded
• ✅ AMVF seeded
• ✅ AMVF (+grid seeder) seeded
• ✅ Track Summary CKF seeded
• ✅ Seeding orthogonal
• ✅ CKF orthogonal
• ✅ IVF orthogonal
• ✅ AMVF orthogonal
• ✅ Track Summary CKF orthogonal
• ✅ Ambisolver seeded
• ✅ Ambisolver orthogonal
• ✅ Seeding ttbar
• ✅ CKF ttbar
• ✅ Ambisolver
• ✅ Track Summary CKF ttbar
• ✅ AMVF ttbar
• ✅ AMVF (+grid seeder) ttbar
• ✅ Truth tracking (GSF)
• ✅ Truth tracking
• ✅ Particles fatras
• ✅ Particles geant4

[paulgessinger]

Hm, shouldn't the ns literal then give us the conversion factor into mm, so that you can write ns and it just does the right thing?

[andiwand]

Hm, shouldn't the ns literal then give us the conversion factor into mm, so that you can write ns and it just does the right thing?

the thing is that we are mixing ns and mm in the output files. we should either consistently use ns or mm. I opted for mm for now as it is also the native internal unit for time

[paulgessinger]

Right and the way in principle that the unit system is designed is that it converts to our internal units. So in my opinion, if the internal unit is mm, when you write 25_ns it should convert this to the equivalent length in mm and if you want ns back, you divide a time by the constant.

[andiwand]

Right and the way in principle that the unit system is designed is that it converts to our internal units. So in my opinion, if the internal unit is mm, when you write 25_ns it should convert this to the equivalent length in mm and if you want ns back, you divide a time by the constant.

yes this is what it does. I am not touching this. I just noticed that we use different units for time when we write it to file. sometimes we do no conversion which results in mm and sometimes we convert to ns. I would argue that this should be consistent to reduce confusion. and I would argue that mm would be easier as we would not have to convert everything including covariance matrices

[beomki-yeo]

Sorry to jump in but I strongly suggest using one of time units (ps ns us ms s) for time components

[beomki-yeo]

Just to demonstrate how it can go wrong in deriving $c=1$ in the natural unit

$$c = \frac{\textrm{distance}}{\textrm{time}} = \frac{299792458 * \textrm{unit::m}}{1 * \textrm{unit::s}} = \frac{299792458000}{299792458000} $$

$$c = \frac{\textrm{distance}}{\textrm{time}} = \frac{299792458 * \textrm{unit::m}}{1 * \textrm{unit::mm} ?} = 299792458000? $$

[andiwand]

not sure if I can follow. I don't suggest to change our unit system.

our units are defined as

constexpr double mm = 1.0;
constexpr double s = 299792458000.0;
constexpr double c = 1.0;

but in examples we are mixing time I/O with mm and ns. I suggest to use one of both and personally I would use mm as it matches up with out internal representation and it might be less surprising during debugging.

but I am happy to use ns if you see a benefit.

note that our digitization config also uses mm for time

[beomki-yeo]

I don't suggest to change our unit system.

I never tried to change the unit system but I am trying fully obey it

Of course we should not mix ns and mm for time -- I am not raising a question on this.

What I suggest is not using length unit for time component which will confuse things a lot as I demonstrated above.
Just try to obtain $c=1$ from using length units in time components...

I will be happy to answer if you don't undertstand why $c$ (speed of light) should be equal to 1

[paulgessinger]

Should we discuss this in the meeting on Tuesday?

[andiwand]

c = 1 is a definition which ties our time unit to the space unit. not sure why you want to derive that

sure lets discuss on tuesday

[beomki-yeo]

Discussing why we should use time unit for time quantity - seriously? Yeah let's discuss it then

c = 1 is a definition which ties or time unit to the space unit. not sure why you want to derive that

This change in this PR is defying the definition of $c=1$. That's my point

[beomki-yeo]

As concluded in the meeting today, let's go with the unified representation for length and time

Invalidated by push of 4e929f8