Time to release a new version?

@roryyorke

Hi,

with all the merged and pending PRs that would be ready to merge, I think it is warranted to take Slycot to the next release. For example, further development of python-control/python-control#376 would need Slycot's support of complex matrices.

@roryyorke are you still around? Any other maintainer? I am also willing to help.

@roryyorke

I can do the merges and release if needed, but it would be much better for @roryyorke and/or @repagh to take a look since they are the people maintaining slycot (and figuring out how to make sure it builds in conda-forge, which seems to be quite non-trivial).

@bnavigator

There are a number of outstanding PRs. I'd ask for @bnavigator's help to review them, but they're almost all his :).

I see #73 has a number of unaddressed remarks - shall we defer this? I don't know if python-control needs mb03rd. @repagh , do you want to take a look at this?

#80 looks experimental, and related to CI; I think we can defer this too.

If the above is right, the PRs to merge are:

It looks like #79 is pretty much good to go, I'll start with a quick review there, and merge it in,~~

@bnavigator , do you know if all of these work together? Do you have a branch where they're all merged in?

We can review the issues (most look like the usual build-difficulty) after we merge these.

So, somewhat embarrasingly, I can't build slycot. This is my build script:

#!/bin/sh
set -ex
export PAGER=

(cd slycot && git log -1)
(cd slycot && git status)

conda --version
conda build --version

conda build -c conda-forge --python=3.8 slycot/conda-recipe-openblas

CMake doesn't find BLAS:

  CMake Error at /home/rory/.miniconda3/conda-bld/slycot_1586503830485/_h_env_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_plac/share/cmake-3.17/Modules/FindPackageHandleStandardArgs.cmake:164 (message):
    Could NOT find BLAS (missing: BLAS_LIBRARIES)

I see this line in .travis.yml:

      export LIBRARY_PATH="$HOME/miniconda/envs/test-environment/lib"

which I'll try to add now.

While looking at that, I see we install scipy and matplotlib for slycot CI:

      conda create -q -n test-environment \
            python="$SLYCOT_PYTHON_VERSION" \
            pip coverage pytest-cov \
            numpy scipy matplotlib \
            $conda_blas

Do we need them?

rory@rory-latitude:~/src/slycot/slycot$ grep -r scipy *
examples.py:    from scipy.linalg import eigvals
tests/test_sg02ad.py:        # https://github.com/scipy/scipy/issues/2251
rory@rory-latitude:~/src/slycot/slycot$ grep -r matplotlib *

That example needs generalized eigenvalues, so numpy.linalg.eigvals isn't good enough.

It doesn't look like we need matplotlib (which, at least on Linux, depends on Qt), though.

This script suceeds:

#!/bin/sh
set -ex
export PAGER=

(cd slycot && git log -1)
(cd slycot && git status)

python3 -m venv --clear venv-build-slycot
source venv-build-slycot/bin/activate
pip install -U pip
pip install scikit-build cmake numpy
(cd slycot && python setup.py install)
python -c "import slycot; print(slycot._wrapper)"

Replacing python setup.py install with pip install . fails, apparently here:

  CMake Error: The source "/tmp/pip-req-build-dmsubg0d/CMakeLists.txt" does not match the source "/home/rory/src/slycot/CMakeLists.txt" used to generate cache.  Re-run cmake with a different source directory.
    File "/tmp/pip-build-env-w1oj5yw4/overlay/lib/python3.6/site-packages/skbuild/setuptools_wrap.py", line 574, in setup
      languages=cmake_languages
    File "/tmp/pip-build-env-w1oj5yw4/overlay/lib/python3.6/site-packages/skbuild/cmaker.py", line 232, in configure
      os.path.abspath(CMAKE_BUILD_DIR())))

We don't currently give pip install . as an installation method in README.rst, though it would be nice if it was supported (that's why we have pyproject.toml, right?). Anyway, for now I can at least do a basic build.

a git clean -fxd removed setup.cfg and _skbuild (and other odds and ends) and now pip install . works; conda build still fails with the same error.

conda build -c conda-forge --python=3.8 slycot/conda-recipe-openblas

CMake doesn't find BLAS:

  CMake Error at /home/rory/.miniconda3/conda-bld/slycot_1586503830485/_h_env_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_plac/share/cmake-3.17/Modules/FindPackageHandleStandardArgs.cmake:164 (message):
    Could NOT find BLAS (missing: BLAS_LIBRARIES)

Hm, setting the BLA_VENDOR="OpenBLAS" in the build.sh should take care of that. In Travis, openblas and other build dependencies are installed explicitly before running conda build. Maybe something wrong with the dependency declaration in meta.yaml?

@bnavigator

@bnavigator , do you know if all of these work together? Do you have a branch where they're all merged in?

They are all based on master. Some might create minor merge conflicts, because they have the same change like removing the suite() lines in test files and removing mathematical.pyf from CMakeLists.txt. Should be easy to resolve if git does not resolve the conflicts automatically.

While looking at that, I see we install scipy and matplotlib for slycot CI:

[...]

Do we need them?

Scipy wasn't needed by Slycot's own unit tests until now. The current 0.3.5 OpenSUSE package builds without it. However, #96 introduces the use of scipy.linalg.eig, too. The python-control tests that we run in Travis CI definitely need it. Matplotlib (probably) too.

Ah, I forgot about running the python-control test suite as part of the slycot tests, thanks.

In https://github.com/roryyorke/Slycot/tree/rory/rebase-merge, the commits 0ded75f and 46769a9 are duplicates of f6494b8 and 91596c1. They belong to #88. but come between #96 and #97.

And f4c4277 belongs to #96 but comes after the commit 158c772 for #97

Please use the branches bnavigator/pr* for the rebase and merge as discussed in #84 (comment)

Another update: Because of the changes (and possibly more to come) in #88, The branches for #96 (not lytex/master but bnavigator/pr96-lytex-comples) and #97 (both bnavigator/travis-to-control-pytest and bnavigator/py97-pytest-travis-coveralls) are based on current master again. They should apply cleanly with or without #88.

I'm checking README.rst; it's mostly still OK.

From the Travis build matrix, Slycot is still passing tests on Python 2.7, so it's probably fine to leave it in for this release (I don't think 2.7 support is adding much burden to us?). Python 3.5 is EOL September 2020, (https://devguide.python.org/#status-of-python-branches), though I don't know if we've tested on that.

I don't know about minimum versions for other packages (numpy, scikit-build, cmake). We now need scipy, but only for a test, as I understand; I'll add that.

This needs updating:

You can also use conda to build and install Slycot from source.  ::

    conda build conda-recipe
    conda install --use-local slycot

conda-recipe doesn't exist anymore. I don't know which of the three recipes to recommend, though;

Presumably conda-recipe-apple is only for macOS -- is it the preferred or only option there?
For Windows are both MKL and OpenBLAS working?
For Linux the MKL / OpenBLAS choice is up to the user (both pass on Travis)

At the end we say you can install "plain old" LAPACK from conda; do we have a conda recipe supporting that? If not, should we just drop that paragraph, or is it intended for "python setup.py install" run inside a conda environment with LAPACK installed---in which case, perhaps its worth stating that in full.

E               Failed: Example ab13fd_example produced a warning.
E               Captured output:
E               The stability radius is
E               0.003919647231698771
E               The minimizing omega is
E               0.9896652038894939
E               
E               Captured stderr:
E               
E               Captured warnings:
E               [SlycotResultWarning('\nFailed to compute beta(A) within the specified tolerance.\nNevertheless, the returned value is an upper bound on beta(A);')]

Allowing it in af98e9f

@moorepants

I've at last released v0.4.0.

@moorepants are you (still?) a maintainer for slycot on PyPI? If so, could you please upload it?

@murrayrm I've at last created a PyPI account, my username is roryyorke.

upload to PyPI
file PR with conda-forge feedstock

It looks like I do have permission to upload. I assume you want me to:

git checkout v0.4.0
python setup.py sdist
twine upload dist/slycot-0.4.0.0.tar.gz

Note too that the conda forge pot will create a PR once PyPi is updated.

@roryyorke

@roryyorke: Added you as a maintainer on PyPi.

@moorepants

@moorepants yes please.

I pushed it up.

🎉 yay!

For the next update (probably 0.4.1), could we please drop the fourth digit in the PyPI version? The git tag, the report during the CMake build and the .egg-info all report 0.4.0, but PyPI has 0.4.0.0.

Conda forge package is in.

Conda forge package is in.

The tests will need a rework, though. On Linux and OS X the new test requirements for pytest and scipy are not being found. On Windows no tests are discovered.

I suggest submitting a PR to the feedstock then. I am not familiar with the details of how the tests have changed.

This is the command that runs on the feedstock:

python -c "from slycot import test; test()"

If that doesn't run anymore, that seems like a backwards incompatibility.

python -c "from slycot import test; test()"
If that doesn't run anymore, that seems like a backwards incompatibility.

#134, addressed in #136

I've sent a release notice to python-control-announce, so I think this is all done.

Thanks everyone!

Navigation Menu

Repository navigation

Time to release a new version? #98

81 remaining items

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Time to release a new version? #98

Description

Activity

murrayrm commented on Apr 6, 2020

roryyorke commented on Apr 10, 2020

roryyorke commented on Apr 10, 2020

roryyorke commented on Apr 10, 2020

roryyorke commented on Apr 10, 2020

bnavigator commented on Apr 10, 2020

bnavigator commented on Apr 10, 2020

bnavigator commented on Apr 10, 2020

roryyorke commented on Apr 10, 2020

bnavigator commented on Apr 10, 2020

bnavigator commented on Apr 10, 2020

bnavigator commented on Apr 11, 2020

roryyorke commented on Apr 11, 2020

81 remaining items

bnavigator commented on May 24, 2020

roryyorke commented on May 30, 2020

roryyorke commented on May 30, 2020

moorepants commented on May 30, 2020

moorepants commented on May 30, 2020

murrayrm commented on May 30, 2020

roryyorke commented on May 30, 2020

moorepants commented on May 30, 2020

bnavigator commented on May 30, 2020

moorepants commented on May 30, 2020

bnavigator commented on May 30, 2020

moorepants commented on May 30, 2020

bnavigator commented on May 30, 2020

roryyorke commented on May 31, 2020

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions