Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Strong Typing #58

Open
s9105947 opened this issue Feb 23, 2022 · 3 comments
Open

Strong Typing #58

s9105947 opened this issue Feb 23, 2022 · 3 comments
Labels
enhancement New feature or request

Comments

@s9105947
Copy link

The reference implementation does not specify the used types, which makes portability really hard for implementing simulations.
Some potential conflicts (from the top of my head):

  • numpy datatypes vs. python builtins (float, int...) vs. python native fixed decimals
  • numpy arrays vs. pandas dataframes vs. python native lists vs. python native tuples
  • floats vs. ints (should 1.0 be equal to 1? If yes, should 1.000001 be equal to 1?)

I'd like to suggest that the reference implementation specifies the minimum supported type, and potentially ensures that the given value can be converted to this type, e.g. when a tuple is requested, lists or numpy arrays of correct length are also accepted.

An implementation is free to support more types, but should make that clear by redefining the property, e.g.:

class Species(picmistandard.Species):
    charge_state: typing.Union[float, Decimal, numpy.half, numpy.single, numpy.double, numpy.longdouble] = 1.0

See PEP 484 for type annotations in python.

@dpgrote
Copy link
Member

dpgrote commented Feb 23, 2022

PICMI is very much relying on duck typing. A float argument should be able to take anything that is compatible with a float (including an integer type). If the implementation for a code has a specific need, it should do whatever casting is needed. In WarpX for example, the only requirement is that a float can be converted to a string (using floating point or exponential formatting). All of the types you listed in you charge_state example allow that.

For any arguments that take a sequence, anything that has an indexing operation and a length method should work. Again, if a implementing code has special needs, it should take care to convert to an acceptable type.

@s9105947
Copy link
Author

"duck typing" I think is the right concept to bring up.

So essentially we would expect a check similar to this one to pass:

def check_type_ducky(expected_type: type, checked_value: typing.Any) -> None:
    """
    performs duck-typing based typecheck
    
    Tries to cast checked_value to expected_type:
    If ok passes silently, else raises.

    **Does not modify checked_value.**

    :param expected_type: type that checked_value must be castable to
    :param checked_value: value to be checked
    :raise ValueError: if checked_value is not castable to expected_type
    """
    try:
        expected_type(checked_value)
    except ValueError:
        raise ValueError(f"can't cast to {expected_type}: {checked_value}")

AFAIK there is no python cast method apart from the convention of using the type as a constructor (int(1.3)).

For the specific case of casting float -> int (e.g. 1.3 -> 1) we could issue a warning "int cast: ignoring fractional part"

Which then leaves the question: Do we want such a check?
In a similar gist to #59 I'd think it would be benefitial, but let's have this general discussion there.

@ax3l ax3l added the enhancement New feature or request label Mar 11, 2022
@ax3l
Copy link
Member

ax3l commented Mar 11, 2022

A float argument should be able to take anything that is compatible with a float (including an integer type). If the implementation for a code has a specific need, it should do whatever casting is needed. In WarpX for example, the only requirement is that a float can be converted to a string (using floating point or exponential formatting). All of the types you listed in you charge_state example allow that.

I think that works with strong typing. We can request an arbitrary numeric type but disallow string. That will make implementations also way easier, e.g., they can do range checks on the numeric input without interpreting a string - and don't need to treat different type handling for most parameters.
This is essentially the same as C++ concepts for generic, templated interfaces.

Some arguments we have will stay very generic, e.g., function generators, etc. That is the minority of options though and can start with typing.Any until we make them more strict later on (if every).

Of course, individual codes can add more constrains and "reject" all but a subset of types, if they do not implement them. This should not often be the case, but would also be a valid way to handle a few very generic inputs, let's say, custom hook classes for custom initializers (again, not the majority and not numeric input).

👍 for adding type checks, as discussed last time.

For examples, see

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants