Moebius Transform Operator and Tustin's method for LTI-Systems

[artpelling]

I have this code here that I would like to integrate in order to have Bilinear Transformations of LTI-systems similarly to Matlabs c2d and d2c commands. So far I have implemented Tustin's method and I am looking to extend this to higher order approximations in the future.

I am particularly interested in arbitrary Moebius substitutions of the frequency argument (not only the bilinear transformation) which is why I chose a more functional approach and implemented a MoebiusTransform operator directly. Maybe this can be used elsewhere in the future (I have added the CayleyTransform as an inspiration).

I would love to get some feedback, IMO the method LTIModel.moebius_substitution could use some improvement in both naming and the code. The method substitutes the frequency argument omega by a MoebiusTransform (please see https://ieeexplore.ieee.org/document/489281/ for reference). I feel like I am not using pyMORs structure optimally in the current implementation.

The docstrings are sparse and sketchy, I will obviously add proper ones and tests at a later stage.

[codecov]

Codecov Report

Merging #1614 (fa218ff) into main (90ca18d) will decrease coverage by 11.27%.
The diff coverage is 90.08%.

Impacted Files	Coverage Δ
src/pymor/algorithms/to_matrix.py	88.23% <77.27%> (-10.43%)	⬇️
src/pymor/models/transforms.py	92.53% <92.53%> (ø)
src/pymor/models/iosys.py	80.66% <93.75%> (+0.49%)	⬆️
src/pymordemos/fenics_nonlinear.py	0.00% <0.00%> (-97.30%)	⬇️
src/pymordemos/neural_networks_instationary.py	0.00% <0.00%> (-97.02%)	⬇️
src/pymordemos/neural_networks.py	0.00% <0.00%> (-96.43%)	⬇️
src/pymordemos/neural_networks_fenics.py	0.00% <0.00%> (-92.96%)	⬇️
src/pymor/reductors/neural_network.py	0.51% <0.00%> (-92.21%)	⬇️
src/pymor/tools/io/vtk.py	3.44% <0.00%> (-91.38%)	⬇️
src/pymor/models/neural_network.py	2.27% <0.00%> (-89.78%)	⬇️
... and 74 more

[pymor-lab-hub-bridge]

Hey @artpelling it looks like this PR touches our CI config (and you are not in my contributor safelist), therefore I am not starting a gitlab-ci build for it and it will not be mergeable.

[pymor-lab-hub-bridge]

Hey @artpelling it looks like this PR touches our CI config (and you are not in my contributor safelist), therefore I am not starting a gitlab-ci build for it and it will not be mergeable.

[artpelling]

I have somehow messed up the history again. @pmli if you are okay with the current state, I can rebase on main, squash everything into one commit and convert this to a non-draft PR.

[pmli]

I have somehow messed up the history again. @pmli if you are okay with the current state, I can rebase on main, squash everything into one commit and convert this to a non-draft PR.

Rebasing would also make it easier to review. You don't have to squash it into one commit, e.g., using one commit per file changed would make the git history nicer (also, we often use git commit messages of the form [module] short message, but we do not really enforce it).

[pmli]

It looks to me to be in a good shape. What is left are docstrings and some unit tests.

[artpelling]

I renamed MoebiusTransform to MoebiusTransformation, because apparently the former refers to Möbius' inversion formula. This leads to inconsistent but correct naming of the derived classes.

[pmli]

It would be good to add tests for Moebius transformations and maybe also for to_discrete and to_continuous methods.

Also, now that it's settled that these transformations are not Operators, maybe __mul__ instead of __matmul__ is better? Or would that be confused with the elementwise product of functions?

[pymor-lab-hub-bridge]

Hey @artpelling it looks like this PR touches our CI config (and you are not in my contributor safelist), therefore I am not starting a gitlab-ci build for it and it will not be mergeable.

[pymor-lab-hub-bridge]

Hey @artpelling it looks like this PR touches our CI config (and you are not in my contributor safelist), therefore I am not starting a gitlab-ci build for it and it will not be mergeable.

[pymor-lab-hub-bridge]

Hey @artpelling it looks like this PR touches our CI config (and you are not in my contributor safelist), therefore I am not starting a gitlab-ci build for it and it will not be mergeable.

[pymor-lab-hub-bridge]

Hey @artpelling it looks like this PR touches our CI config (and you are not in my contributor safelist), therefore I am not starting a gitlab-ci build for it and it will not be mergeable.

[renefritze]

I've add you to the safelist now @artpelling , so this message should disappear with the next push/commit.

[sdrave]

@artpelling, the problem with the failing test was that you are not allowed to call as_range_array on an Operator which has something different than a NumpyVectorSpace as source. I have moved the fallback in to_matrix to an additional as_array action. As this should also work for the children in the Concatenation, I do not think it is necessary to catch NoMatchingRuleError.

Further I have reverted your streamlined version of the matrix-matrix products as it seems to neglect one optimization for sparse-dense products.