Skip to content

27 arithmetic operations for parameters #43

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

Merged
merged 72 commits into from
Aug 14, 2024

Conversation

damskii9992
Copy link
Contributor

No description provided.

@damskii9992 damskii9992 added [scope] enhancement Adds/improves features (major.MINOR.patch) feature PR label labels Aug 1, 2024
@damskii9992 damskii9992 linked an issue Aug 1, 2024 that may be closed by this pull request
@rozyczko
Copy link
Member

rozyczko commented Aug 2, 2024

Apart from code review, a few things I found while testing

__radd__ vs. __add__ - should be consistent

    3+3+p -> "6+p"
    p+3+3 -> "p+3+3"

Nan/inf treatment. Are the p+np.inf bounds correct?

>>> p
<Parameter 'a': 1.0000, bounds=[-inf:inf]>
>>> p+np.inf
<Parameter 'a + inf': inf, bounds=[nan:inf]>
>>> p+np.nan
<Parameter 'a + nan': nan, bounds=[nan:nan]>

___div___ issue with the order (probably a scipp issue, or something wrong with __rdiv__ vs. __div__)

>>> a1
<Parameter 'a1': 2.0000 km, bounds=[-inf:inf]>
>>> a3
<Parameter 'a3': 2.0000 nm, bounds=[-inf:inf]>
>>> a1/a3
<Parameter 'a1 / a3': 1000000000000.0000, bounds=[-inf:inf]>
>>> a3/a1
scipp._scipp.core.UnitError: Conversion from `10e-13` to `2.18154854429043519e+244*s^(-13)*A^(-13)` is not valid.

@henrikjacobsenfys
Copy link
Member

b2=Parameter("b2", 2.0)
In [20]: b3=b2+1
In [21]: b3
Out[21]: <Parameter 'b2 + 1': 3.0000, bounds=[-inf:inf]>
In [22]: b3*2
Out[22]: <Parameter 'b2 + 1 * 2': 6.0000, bounds=[-inf:inf]>

Should probably be
<Parameter '(b2 + 1) * 2': 6.0000, bounds=[-inf:inf]>

@damskii9992 damskii9992 merged commit 2444f1b into develop Aug 14, 2024
26 checks passed
@damskii9992 damskii9992 deleted the 27-Arithmetic-operations-for-Parameters branch August 14, 2024 13:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature PR label [scope] enhancement Adds/improves features (major.MINOR.patch)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Arithmetic operations for Parameters and DescriptorNumbers
4 participants