Repository: BAMWelDX/weldx

Issue:
Optimize length calculation of `DynamicBaseSegment`
The length calculation for expressions can actually fail in its current form. Here is an example:

~~~ python
from weldx import Q_, DynamicShapeSegment


class MySegment(DynamicShapeSegment):
    def __init__(self):
        f = "x * sin(s) + y * s"
        p = dict(x=Q_([1, 0, 0], "mm"), y=Q_([0, 1, 0], "mm"))
        super().__init__(f, parameters=p)


MySegment()
~~~

This crashes with the following error:

~~~
Traceback (most recent call last):
  File ".../sympy_bug.py", line 10, in <module>
    
  File ".../sympy_bug.py", line 8, in __init__
    super().__init__(f, parameters=p)
  File "...\weldx\geometry.py", line 144, in __init__
    self._update_internals()
  File "...\weldx\geometry.py", line 151, in _update_internals
    self._length = self.get_section_length(self._max_coord)
  File "...\weldx\geometry.py", line 213, in get_section_length
    length = self._length_expr.evaluate(max_coord=position).data
  File "...\weldx\core.py", line 277, in evaluate
    return self.function(**variables, **parameters)
  File "<lambdifygenerated-2>", line 4, in _lambdifygenerated
NameError: name 'Integral' is not defined
~~~

Change the expression from `x * sin(s) + y * s` to `x * sin(s) + y * cos(s)` and everything works fine. The problem here seems to be that the integration of the length formula gets really complicated and can't be solved by sympy using symbolic calculations. So the integral is kept as a "wrapper" (I guess) to solve it later in a different way. But this can't be turned into a python function (and we do that). So we might need to provide different methods for the length calculation. IIRC, sympy supports different alternatives, numeric integration amongst them. The last resort would be to catch this error and inform the user, that the length couldn't be determined automatically and he has to provide a custom function for that.

Discussion:
marscher: note that the deepsource issues are unrelated to my changes :P
marscher: @vhirtham I've noticed, that using scipy as backend now triggers deprecation warnings like this: 

`DeprecationWarning: scipy.sin is deprecated and will be removed in SciPy 2.0.0, use numpy.sin instead`

However we need to use scipy to make "Integrate" use the right backend. Do you have a suggestion how to proceed?


marscher: xref: https://github.com/sympy/sympy/issues/20294
codecov[bot]: # [Codecov](https://codecov.io/gh/BAMWelDX/weldx/pull/770?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=BAMWelDX) Report
> Merging [#770](https://codecov.io/gh/BAMWelDX/weldx/pull/770?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=BAMWelDX) (14ba130) into [master](https://codecov.io/gh/BAMWelDX/weldx/commit/ceaa4c507eae60f65da6d39059a6634dfd747e39?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=BAMWelDX) (ceaa4c5) will **decrease** coverage by `0.04%`.
> The diff coverage is `100.00%`.

```diff
@@            Coverage Diff             @@
##           master     #770      +/-   ##
==========================================
- Coverage   96.83%   96.78%   -0.05%     
==========================================
  Files          88       88              
  Lines        6009     6012       +3     
==========================================
  Hits         5819     5819              
- Misses        190      193       +3     
```


| [Impacted Files](https://codecov.io/gh/BAMWelDX/weldx/pull/770?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=BAMWelDX) | Coverage Δ | |
|---|---|---|
| [weldx/core.py](https://codecov.io/gh/BAMWelDX/weldx/pull/770/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=BAMWelDX#diff-d2VsZHgvY29yZS5weQ==) | `93.41% <ø> (ø)` | |
| [weldx/geometry.py](https://codecov.io/gh/BAMWelDX/weldx/pull/770/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=BAMWelDX#diff-d2VsZHgvZ2VvbWV0cnkucHk=) | `96.61% <100.00%> (+0.01%)` | :arrow_up: |
| [weldx/asdf/util.py](https://codecov.io/gh/BAMWelDX/weldx/pull/770/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=BAMWelDX#diff-d2VsZHgvYXNkZi91dGlsLnB5) | `91.19% <0.00%> (-0.95%)` | :arrow_down: |

------

[Continue to review full report at Codecov](https://codecov.io/gh/BAMWelDX/weldx/pull/770?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=BAMWelDX).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=BAMWelDX)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/BAMWelDX/weldx/pull/770?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=BAMWelDX). Last update [e36d678...14ba130](https://codecov.io/gh/BAMWelDX/weldx/pull/770?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=BAMWelDX). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=BAMWelDX).

github-actions[bot]: ## Unit Test Results
2 153 tests  +1   2 153 [:heavy_check_mark:](https://github.com/EnricoMi/publish-unit-test-result-action/blob/v1.20/README.md#the-symbols "passed tests") +1   2m 28s [:stopwatch:](https://github.com/EnricoMi/publish-unit-test-result-action/blob/v1.20/README.md#the-symbols "duration of all tests") -6s
       1 suites ±0          0 [:zzz:](https://github.com/EnricoMi/publish-unit-test-result-action/blob/v1.20/README.md#the-symbols "skipped / disabled tests") ±0 
       1 files   ±0          0 [:x:](https://github.com/EnricoMi/publish-unit-test-result-action/blob/v1.20/README.md#the-symbols "failed tests") ±0 

Results for commit 14ba1300. ± Comparison against base commit e36d678d.

vhirtham: > note that the deepsource issues are unrelated to my changes :P

I silenced it for the future ;)

Patch:
diff --git a/CHANGELOG.rst b/CHANGELOG.rst
index 1558f9b..f834d8c 100644
--- a/CHANGELOG.rst
+++ b/CHANGELOG.rst
@@ -2,6 +2,16 @@
  Release Notes
 ###############
 
+********************
+ 0.6.2 (tba)
+********************
+
+fixes
+=====
+
+-  `MathematicalExpression` now uses SciPy and NumPy in numerical function evaluation. This enables it to use
+   advanced integration methods and fixes lengths computation of `DynamicShapeSegment` [:pull:`770`].
+
 ********************
  0.6.1 (19.05.2022)
 ********************
diff --git a/weldx/core.py b/weldx/core.py
index f4ccd6e..1d47876 100644
--- a/weldx/core.py
+++ b/weldx/core.py
@@ -56,7 +56,7 @@ class MathematicalExpression:
         self._expression: sympy.Expr = expression
 
         self.function = sympy.lambdify(
-            tuple(self._expression.free_symbols), self._expression, "numpy"
+            tuple(self._expression.free_symbols), self._expression, ("numpy", "scipy")
         )
 
         self._parameters: dict[str, Union[pint.Quantity, xr.DataArray]] = {}
diff --git a/weldx/geometry.py b/weldx/geometry.py
index 15fcf05..2540c50 100644
--- a/weldx/geometry.py
+++ b/weldx/geometry.py
@@ -2,6 +2,7 @@
 from __future__ import annotations
 
 import copy
+import warnings
 from dataclasses import InitVar, dataclass
 from pathlib import Path
 from typing import TYPE_CHECKING, Union
@@ -211,7 +212,14 @@ class DynamicBaseSegment:
 
         """
         if self._series.is_expression:
-            length = self._length_expr.evaluate(max_coord=position).data
+            # ignore unit stripped warning, as it is expected.
+            # Note, that the result still has the unit,
+            # it is stripped during computation (scipy.integrate)
+            with warnings.catch_warnings():
+                warnings.filterwarnings(
+                    "ignore", category=pint.errors.UnitStrippedWarning
+                )
+                length = self._length_expr.evaluate(max_coord=position).data
         else:
             length = self._len_section_disc(position=position)
         if length <= 0:


Test patch:
diff --git a/weldx/tests/test_core.py b/weldx/tests/test_core.py
index 5952af9..b3ea4f0 100644
--- a/weldx/tests/test_core.py
+++ b/weldx/tests/test_core.py
@@ -199,6 +199,20 @@ class TestMathematicalExpression:
         with pytest.raises(exception_type):
             ma_def.evaluate(**variables)
 
+    @staticmethod
+    def test_integrate_length_computation():
+        """Ensure we can integrate with Sympy during length computation."""
+        from weldx import DynamicShapeSegment
+
+        class MySegment(DynamicShapeSegment):
+            def __init__(self):
+                f = "x * sin(s) + y * s"
+                p = dict(x=Q_([1, 0, 0], "mm"), y=Q_([0, 1, 0], "mm"))
+                super().__init__(f, parameters=p)
+
+        s = MySegment()
+        assert s.get_section_length(1).u == Q_("mm")
+
 
 # --------------------------------------------------------------------------------------
 # TimeSeries
@@ -468,7 +482,7 @@ class TestTimeSeries:
     @pytest.mark.parametrize(
         "time,  exception_type, test_name",
         [
-            # (DTI(["2010-10-10"]), ValueError, "# wrong type #1"),
+            # (DTI(["2010-10-10"]), ValueError, "# wrong type #1"),  # skipcq: PY-W0069
             ("a string", TypeError, "# wrong type #2"),
             ([1, 2, 3], TypeError, "# wrong type #3"),
             (1, TypeError, "# wrong type #4"),