Units in models and parsers

[NicolasGensollen]

A few unit related problems I noticed:

• Unit conversions are currently managed in the parsers (most of the time hardcoded when setting the attribute value), which is hard to maintain.

Ex:

#From cyme reader
api_winding.nominal_voltage=float(settings['primarybasevoltage'])*10**3 #DiTTo in volt

• Attribute units are not clear when looking at the DiTTo model. Sometimes there is some indication in the help but not always. In addition some names are misleading in terms of the unit assumed by DiTTo.

Ex:

#In KVA or VA???
normhkva = Float(help='Normal maximum kVA rating for H winding', default_value=None)

These are already introducing some bugs in the conversions and it will probably get worse as we add more parsers...

Having a more robust framework to handle units (like Pint maybe??) would be a nice addition in my opinion. Thoughts?

[kdheepak]

Unfortunately, as a general statement, Python doesn't have a good solution for units. Though, for our specific use case we may be able to find something that works. Pint may not play well with traitlets, I haven't tested it but from looking at the source code it seems like both Pint and traitlets make heavy use of descriptors and subclass constraints (using __new__), and that may mean that they may not compose well with each other. I would first experiment with using callbacks or validation in traitlets to see if you can use that to solve your problem [1].

[1] https://traitlets.readthedocs.io/en/stable/api.html#callbacks-when-trait-attributes-change

[NicolasGensollen]

@tarekelgindy and I had a discussion on this subject this morning. The following is a possible solution we came up with:

Change the models and add a unit and a unit_type keyword:

For example, in wire.py:

X = Float(help="x coordinate of the wire", default=0, unit="m", unit_type="distance")

In the readers, the attribute setting should be changed to:
read.py

#The input gives a x coordinate of 10 feet. The user doesn't have to worry about conversion
api_wire.set("X", 10, "ft") 

The set function has to be implemented in the base class. It probably has to check that X is a valid attribute of wire object, and that "ft" is a valid unit for this attribute (this is why we have a unit_type keyword which maps the type of unit to a set of authorized units: "distance": set("m","km","ft","kft","mi",...)). The set method then does the conversion from feet to meters using Pint or something else.
This way, if we decide to change the way X is represented in DiTTo, we simply change unit="m" by unit="kft" for example.

What are your thoughts on this @kdheepak? Do you think we could implement something like that?

[kdheepak]

It might work well. Why do we need unit_type?

[kdheepak]

I'll also need to prototype this or see a prototype and experiment to see if it works as intended.

[NicolasGensollen]

We might need unit type to avoid stuffs like:

api_wire.set("X",10,"kva") #KVA is not a valid unit for length

There might be other ways to do that though...

[NicolasGensollen]

I'm trying to see what I can do with that but I have troubles getting the attributes' kwargs in the DiTToHasTraits class where I am implementing the set function. I need to get the unit information defined in wire.X for example. I can get the values using _trait_values but not the other arguments (help, unit, unit_type...)

This is what I get when I print self.__dict__ in DiTToHasTraits.

'_trait_validators': {}, '_model': <ditto.store.Store(elements=0, models=1) object at 0x10c15e790>, '_trait_values': {'diameter': None, 'Y': None, 'insulation_thickness': 0.0, 'is_fuse': None, 'resistance': None, 'is_network_protector': 0, 'is_open': None, 'concentric_neutral_gmr': None, 'ampacity': None, 'is_switch': None, 'gmr': None, 'emergency_ampacity': None, 'concentric_neutral_diameter': None, 'concentric_neutral_resistance': None, 'phase': None, 'X': 10.0, 'response': None, 'interrupting_rating': None, 'drop': 0, 'nameclass': None, 'is_breaker': None, 'is_recloser': None}, '_cross_validation_lock': False, '_trait_notifiers': {}}

I tried, with no success, modifying the Float class to store this information:

class Float(T.Float, DiTToTraitType):
    def __init__(self,**kwargs):
         if "unit" in kwargs:
             self.unit=kwargs["unit"]
         super.__init__(self)

Since I've never really worked with traitlets before, I'm not sure how to do this. Any idea?
Hope that makes sense...

[kdheepak]

I'm able to do something like this:

import traitlets as T

class Float(T.Float, T.TraitType):
    def __init__(self, **kwargs):
         if "unit" in kwargs:
             self.unit=kwargs.pop("unit")
         super().__init__(**kwargs)

class Test(T.HasTraits):
    x = Float(unit="m")


t = Test(x=1)
print(t.x)
print(Test.x.unit)

but I still need to think about this more before we commit to doing it this way. Specifically, the set method is not ideal. And I still don't see a strong need for unit_type. That may be valuable for developers but I don't see it valuable for users.

[NicolasGensollen]

Thanks for having a look @kdheepak , I'll try again then.
We might not need to expose unit_type to the user but I think we need to have a way to check that the units given by the user make sense.
I'm changing the priority of this since it is creating a lot of errors currently.