MPh-py / MPh

Pythonic scripting interface for Comsol Multiphysics
https://mph.readthedocs.io
MIT License
265 stars 67 forks source link

Add units support with Pint #165

Closed elyurn closed 5 months ago

elyurn commented 5 months ago

Hi!

I would like to know if you would be interested in supporting unit management and direct formatting to COMSOL with Pint? The idea would be to define a variable with its unit, and be able to directly import it as is in COMSOL. For example, one could to something like that:

import pint
si = pint.UnitRegistry()

@pint.register_unit_format("comsol")
def format_unit_simple(unit, registry, **options):
    return "["+"*".join(f"{u}^{p}" for u, p in unit.items())+"]"
si.default_format = "~comsol"

a = 2*si.m
model.parameter("a", str(a))
john-hen commented 5 months ago

Hi, I'm not sure about this. I like the idea of supporting units, but only if it's not forced on anyone, i.e. maintains backward compatibility. I cannot tell if that's the case here based on this one example.

I often add some kind of unit handling to my own code. It helps avoid simple mistakes in calculations. The simplest way to do it is just extra factors, like m = 1 for meters and km = 1000*m and so on. That's very ad-hoc. Using an established and flexible library such as Pint is certainly the better way to do it.

Your example doesn't seem to require any integration with MPh. You can just do that in application code. But I guess that's not the full story? There are also API methods where we return values, either as strings including units, or as numbers in implicit units (defaults units or units as requested). Such as model.parameter() and model.evaluate(). I don't, at this point, see how those would fit into the picture.

elyurn commented 5 months ago

I totally agree with you on this:

only if it's not forced on anyone, i.e. maintains backward compatibility.

In my opinion, the advantage of having a package like Pint is to mimic COMSOL's warning when adding incompatible units, but directly in Python. I agree that it's not really related to MPh and it could be given as an example of good practice (i.e. like it's done in the docs with multiprocessing).

But I guess that's not the full story?

For my simple usage, it actally is. So you can close the issue :)

There are also API methods where we return values, either as strings including units, or as numbers in implicit units (defaults units or units as requested). Such as model.parameter() and model.evaluate(). I don't, at this point, see how those would fit into the picture.

I don't see any problem with this. I just think that handling units management is a cleaner way to evaluate results. We can have acess to the variable's magnitude and unit with the appropriate Pint methods.