branch off new slycot version

[artpelling]

I saw that a new slycot version was released last week. It includes my bugfix PR for an incorrectly set ldwork variable in the discrete-time case. If we update our requirements to version 0.5.0 or higher, we can remove the workaround bugfix from our bindings.

The new release does not seem to contain drastic changes, although they have switched to a different license and version of SLICOT and there seems to be an issue with NumPy 1.23.0 (but not 1.23.1 and greater).

https://github.com/python-control/Slycot/releases/tag/v0.5.0

[renefritze]

Will the current workaround break with 0.5.0? Could we branch the code on the slycot version instead?

[pmli]

Will the current workaround break with 0.5.0?

No, it shouldn't.

Could we branch the code on the slycot version instead?

That might be better, to support more Slycot versions. Or we could postpone merging this PR until there are a few more releases of Slycot, in a year or two.

[renefritze]

Will the current workaround break with 0.5.0?

No, it shouldn't.

Great, so there's no rush to get this in before the release.

Could we branch the code on the slycot version instead?

That might be better, to support more Slycot versions. Or we could postpone merging this PR until there are a few more releases of Slycot, in a year or two.

I would prefer the version branch for now. We can add a deprecation warning for slicot 0.4 sometime after.

That sound good @artpelling, @pmli ?

[artpelling]

That sound good @artpelling, @pmli ?

Yes sounds good, will get on that soon. This really is low priority, everything should continue to work fine even if we do nothing. :)

[renefritze]

Maybe you could also look at the warnings we get from slycot when you get to this? Starting in line 1477 conda.log.txt

[artpelling]

@renefritze I just wanted to test the changes locally, but all the lyapunov related tests are skipped, because I don't have pymess. The decorators

@skip_if_missing('SLYCOT')
@skip_if_missing('PYMESS')

make it such that the tests are skipped if either package is missing. What would be the best way to change this behaviour? Should I conditionally edit the solver lists in pymortests/lyapunov.py?

For now, I worked around it and tests pass for all slycot versions.

[renefritze]

Sorry @artpelling, I tried to contact you on gitter. Yes, conditionally setting the solver backends would be my solution as well. And then removing the skip decorators altogether.

[pmli]

Maybe you could also look at the warnings we get from slycot when you get to this? Starting in line 1477 conda.log.txt

@lbalicki might know what to do about the issue with the unstable_heat demo:

src/pymortests/demos.py::test_demos[unstable_heat:'50 10']
  /usr/share/miniconda3/envs/pyMOR-ci/lib/python3.8/site-packages/slycot/exceptions.py:241: SlycotResultWarning:
  The computed solution may be inaccurate due to poor
  scaling or eigenvalues too close to the boundary of
  the  imaginary axis.

[renefritze]

Just fyi: if you rebase on main, the newer slycot version will also be included in the CI images for gitlab CI.

[artpelling]

I guess this can be merged now? Do we have some workflow for reminding ourselves to add deprecation warnings for Slycot in 2023.1 or something?

[renefritze]

I guess this can be merged now?

Let's check the log in a bit if this works in Gitlab too.

Do we have some workflow for reminding ourselves to add deprecation warnings for Slycot in 2023.1 or something?

Not explicitly. Just create a new issue and add that to the release milestone?

[codecov[bot]]

Codecov Report

Merging #1689 (57ac9e0) into main (7c8bbba) will decrease coverage by 5.78%. The diff coverage is 50.00%.

Additional details and impacted files

Impacted Files	Coverage Δ
src/pymor/bindings/slycot.py	80.64% <50.00%> (-2.69%)	:arrow_down:
src/pymordemos/neural_networks.py	0.00% <0.00%> (-96.43%)	:arrow_down:
src/pymordemos/fenics_nonlinear.py	0.00% <0.00%> (-96.06%)	:arrow_down:
src/pymor/models/neural_network.py	2.27% <0.00%> (-92.05%)	:arrow_down:
src/pymordemos/neural_networks_fenics.py	0.00% <0.00%> (-91.55%)	:arrow_down:
src/pymordemos/neural_networks_instationary.py	0.00% <0.00%> (-85.82%)	:arrow_down:
src/pymor/reductors/neural_network.py	0.42% <0.00%> (-85.54%)	:arrow_down:
src/pymortests/mpi_run_demo_tests.py	2.12% <0.00%> (-78.73%)	:arrow_down:
src/pymor/discretizers/fenics/cg.py	0.00% <0.00%> (-75.87%)	:arrow_down:
src/pymor/parallel/mpi.py	26.47% <0.00%> (-73.53%)	:arrow_down:
... and 58 more