I noticed that when misspelling a particular unit, i.e. writting Gev instead of GeV the units are not properly recognised.
Shall we keep them case sensitive for some reason? Or do we have the choice?
I think having them case insensitive would better the user experience, but perhaps having more explicit error message when this happens can be enough, I guess the units should technically be case sensitive, but in practice for our use case its not really necessary.
I disagree about allowing case insensitive units. Instead proper error messages should be raised for unknown units.
Otherwise all of a sudden you run into issues like mev and similar. Note that this isn’t an academic example, because REST might describe MeV events (e.g. energy deposition of muons in a few cm scintillator vs. expected axion masses in the milli electronvolt range!
In general it is more important to disallow things explicitly than allowing too many things (that’s where undefined behavior sneaks in often).
The problem is that with too much output the user usually ignores the warning, even if it is highlighted in another color. So, should we just terminate the execution with an error till the problem is fixed?
Yes, I’m arguing to throw an exception if the unit is wrong.
Could you implement the exception?
I suppose that would be much less work for you, because I’ve never looked at the unit parser before. No idea where that is in REST. In principle I can do it, but it’d take me probably an hour or so, which I might only have to spare after next Tuesday.
I don’t know how smart the unit parser currently is, but given that you ask the things in the opening post I assume rather simple so far. In general imo it’d be good to have a proper unit parser that can automatically parse SI prefixes + units. Then one only needs to define units one wishes to support, without specifying possible prefixes. That gives you automatic conversion to the desired internal REST units for a certain process.
Something like the parser of: GitHub - SciNim/Unchained: A fully type safe, compile time only units library. (well, unfortunately you don’t get the nice CT only properties. 
)
1 Like
Well, I am asking to learn how do you implement the exception, I am not familiar with . The approach we are following is to just print out an error message and exit(N).
I believe this should be done at the IsUnit implementation at framework/TRestSystemOfUnits.cxx at master · rest-for-physics/framework · GitHub
///////////////////////////////////////////////
// \brief Checks if the string is a REST supported unit
///
/// REST supports several basic units and kinds of their combinations(multiply, divide)
/// e.g.
/// cm, ns, mV, kg, keV
/// V/cm, kg-yr
///
/// Note: REST doesnt support units combination with numbers, e.g. m/s^2
bool IsUnit(string unitsStr) { return !TRestSystemOfUnits(unitsStr).IsZombie(); }