Repository navigation
Add es.math maths module with uncertainty and bounds - #329
markbujehein wants to merge 141 commits into
Conversation
…odule Files as of origin/add-maths-tutorials (71ea6a2), unchanged. The shared files keep both sides: the module branch's notebooks, nav entries and user-guide pages, plus the tutorial navigation, the Concepts page and the recursive notebook globs. Updates for the current API follow separately. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ease-3.0 API
- display_name instead of name, global_object.registry instead of map, and
Sampler.from_fitter instead of passing a fitter to Sampler.
- Inverse trig results are converted with convert_unit('deg') instead of the
removed unit= keyword.
- erf, erfc and arctan2 refuse numbers with an uncertainty: the notebooks show
the VariancesError, use exact inputs, and propagate the erf error by hand.
The arctan2 exercise and chart use arctan(b / a) for a positive real part.
- tan near a pole is no longer refused: the notebook shows the huge value.
- Expected error messages follow the new "is not finite" wording.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
markbujehein
changed the title
removed strings in minimizer indexing mechanism #347
Import Minimizers instead of AvailableMinimizers
4cf689d to
1643267
Compare
| asin = arcsin | ||
| acos = arccos | ||
| atan = arctan | ||
| atan2 = arctan2 | ||
| asinh = arcsinh | ||
| acosh = arccosh | ||
| atanh = arctanh |
There was a problem hiding this comment.
Do we need these? Just because the functions are spelled a certain way in Scipp does not mean we have to have the same spelling in EasyScience.
| Self | ||
| A new instance of the class this is called on. |
There was a problem hiding this comment.
You never fixed this . . .
| # math must come after base_classes, which has an import cycle with variable | ||
| from . import math # noqa: E402 # isort: skip | ||
|
|
There was a problem hiding this comment.
This is redundant, it's already in this module
| Unlike ``arctan(y / x)``, this uses the signs of both coordinates, | ||
| so the angle lands in the right quadrant. ``y`` and ``x`` must have | ||
| the same unit, and the result is in ``rad``. Scipp cannot propagate | ||
| an uncertainty through this function yet, so neither input may have | ||
| a variance. |
There was a problem hiding this comment.
I would argue we shouldn't have this function yet then. It is confusing that some of the functions in the module propagates variance and others don't. You can make an issue on it and link to the Scipp issue, (assuming there is one?)
| >>> import easyscience as es | ||
| >>> x = es.Parameter(1.0, display_name='x', variance=0.01, min=0.0, max=2.0) | ||
| >>> print(es.math.tanh(x)) | ||
| <Parameter 'tanh(x)': 0.7616 ± 0.0420, bounds=[0.0:0.9640275800758169]> |
There was a problem hiding this comment.
Again, please use the properly formatted python markdown when writing examples . . .
```python
Your code snippet goes here, without the >>>
```
| def _monotonic_bounds(function_values: Callable) -> _ResultBoundsFunction: | ||
| """ | ||
| Result bounds of a monotonic function: its values at the argument | ||
| bounds. | ||
| """ | ||
|
|
||
| def bounds(argument_bounds: _Bounds) -> tuple[float, float]: | ||
| lower, upper = argument_bounds | ||
| at_lower, at_upper = float(function_values(lower)), float(function_values(upper)) | ||
| return min(at_lower, at_upper), max(at_lower, at_upper) | ||
|
|
||
| return bounds |
There was a problem hiding this comment.
Why make this so complicated? You're essentially just saying that the output should have:
temp_param = func(param.value)
temp_param.min = min(func(param.min), func(param.max))
temp_param.max = max(func(param.min), func(param.max))But you're hiding it away behind a dataclass storing a function which is computed from a function which takes the callable as input . . . .
This is a good example of where code becomes too DRY. Better to write those two lines I wrote above for every function, than to abstract it away to this degree, it is much harder to find out what actually happens in each function this way . . .
| for arg in args: | ||
| if not isinstance(arg, (DescriptorNumber, DescriptorArray)): | ||
| raise TypeError( | ||
| 'EasyScience math functions support `Parameter`, `DescriptorNumber` and ' | ||
| f'`DescriptorArray` objects, not {type(arg)}.' | ||
| ) |
There was a problem hiding this comment.
Why do here accept multiple inputs? You haven't even typehinted that you accept multiple inputs . . .
These functions should only take one input for now. YAGNI: You Are Not Gonna Need It.
We can implement multiple input WHEN and IF we need it, for now Keep It Simple.
| def _scipp_value(descriptor: DescriptorNumber | DescriptorArray) -> sc.Variable: | ||
| """ | ||
| Scipp value of a descriptor, without variances if they are all zero. | ||
|
|
||
| A ``Parameter`` stores a variance of 0.0 by default. Scipp would | ||
| propagate it to ``nan`` where the derivative is infinite, and its | ||
| ``erf``, ``erfc`` and ``atan2`` refuse any variance, so a zero | ||
| variance is dropped. | ||
| """ | ||
| value = descriptor.full_value | ||
| if value.variances is not None and not np.any(value.variances): | ||
| return sc.values(value) | ||
| return value |
There was a problem hiding this comment.
I'm surprised you haven't mentioned anything. It seems obvious that we should consider changing the default variance to None rather than 0.0 on Parameter. If we did, you would also avoid this issue.
| @dataclass(frozen=True) | ||
| class _InputRange: | ||
| """The argument values a function is defined for.""" | ||
|
|
||
| lower_limit: float = -np.inf | ||
| upper_limit: float = np.inf | ||
|
|
||
| def clip(self, lower: float, upper: float) -> _Bounds: | ||
| """Restrict the bounds ``[lower, upper]`` to this range.""" | ||
| return max(lower, self.lower_limit), min(upper, self.upper_limit) | ||
|
|
||
|
|
||
| @dataclass(frozen=True) | ||
| class _MathFunction: | ||
| """ | ||
| What a public maths function evaluates and how its result bounds are | ||
| found. | ||
|
|
||
| Attributes | ||
| ---------- | ||
| name : str | ||
| Public name, used in error messages and in the name of the | ||
| result. | ||
| scipp_function : Callable | ||
| Scipp function that evaluates the value and the variance and | ||
| checks the units. | ||
| result_bounds : _ResultBoundsFunction | ||
| Takes the bounds of each argument and returns the bounds of the | ||
| result. | ||
| input_range : _InputRange, default=_InputRange() | ||
| Argument values the function is defined for. Argument bounds are | ||
| clipped to it before ``result_bounds`` runs. | ||
| bounds_unit : str | None, default=None | ||
| Unit that ``result_bounds`` expects the argument bounds in: | ||
| ``'rad'`` for the trigonometric functions, whose NumPy versions | ||
| take radians. | ||
| """ | ||
|
|
||
| name: str | ||
| scipp_function: Callable | ||
| result_bounds: _ResultBoundsFunction | ||
| input_range: _InputRange = _InputRange() | ||
| bounds_unit: str | None = None |
There was a problem hiding this comment.
I wouldn't make the math functions dataclasses. Dataclasses are meant to be for small classes which just holds data. Yours holds functions and logic.
It'd be simpler to just use functions. Right now, your actual math functions have no logic in them, they're just a docstring and a call to the dataclass, which makes the code more difficult to read. Have code where it is used unless you ABSOLUTELY have to move it out. And always start with the simplest solution (i.e. regular functions and classes), only move on to more advanced solutions when you have a good REASON to do so.
Summary
Adds
es.math, so maths functions can be applied to Parameters and keep their units, uncertainties and fit bounds, e.g.The functions are
sin,cos,tan,sinc,arcsin,arccos,arctan,arctan2,sinh,cosh,tanh,arcsinh,arccosh,arctanh,exp,log,log10,sqrt,erfanderfc, plus the common aliasesasin,acos,atan,atan2,asinh,acoshandatanh(all found in Scipp).They work on
Parameter,DescriptorNumberandDescriptorArray; output takes same type as the input.Scipp now handles almost everything; this module adds bounds, but features less guardrails compared to earlier (allowing Scipp-related issues to show instead of trying to patch them).
The PR now also features tutorials (previously #331)
Motivated by
Solves #201.
How it works
sinover[0, 90] deggives[0, 1].nanor infinite raisesValueError, for examplelogof a negative number.Other changes
DescriptorNumber.from_scippreturnsSelf, so it type-checks on subclasses.docs/api-reference/math.md. Every function has a tested example.>>>primary prompts are used. I was urged to try out some custom styling, so I've done that. Docstring examples can also be done in Markdown, but then please give me a standard.Tests
tests/unit/variable/test_math_examples.py: verifies the maths inside the docstring examples.Known limitations
erf,erfcandarctan2fail on inputs with an uncertainty, because Scipp doesn't support it for them. That includes all fitted parameters. The tutorials show this and work around it.tannear π/2 returns a huge number instead of an error. The trigonometry tutorial warns about it.parameter.pyand should perhaps be a shared constant.