bnavigator commented 4 years ago

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 https://github.com/python-control/python-control/pull/376 would need Slycot's support of complex matrices.

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

murrayrm commented 4 years ago

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).

roryyorke commented 4 years ago

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:

[x] #79
[x] #82
[x] #83
[x] #84
[x] #87
[x] #88
[x] #96
[x] #97
[x] #101
[x] #103
[x] #104
[x] #105
[x] #110
[x] #112
[x] #114
[x] #115
[x] #116

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.

roryyorke commented 4 years ago

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.

roryyorke commented 4 years ago

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.

roryyorke commented 4 years ago

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.

bnavigator commented 4 years ago

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 commented 4 years ago

@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.

bnavigator commented 4 years ago

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.

roryyorke commented 4 years ago

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

bnavigator commented 4 years ago

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

And f4c42774d9b7efcb13b5c93598365d3b859ad83a belongs to #96 but comes after the commit 158c772dc75db7e3ea6f8de58bfde207408ee5b4 for #97

bnavigator commented 4 years ago

Please use the branches bnavigator/pr* for the rebase and merge as discussed in https://github.com/python-control/Slycot/pull/84#issuecomment-61213864

bnavigator commented 4 years ago

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.

roryyorke commented 4 years ago

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.

bnavigator commented 4 years ago

On Python2: I'd say we leave it in as long as the CI builds it fine.

bnavigator commented 4 years ago

Travis CI configuration in #97 builds both 2.7 and 3.5 versions.

On the conda-recipes @repagh should have a word, as he knows what is going on in conda-forge feedstock.

MKL is conda only.

IIRC, "Generic" i.e. "plain old" LAPACK did not work in conda at all when I made the reorganization in #90 and #93. So unless this is different now, we should remove that paragraph at the end.

conda-recipe-apple is the only configuration we got working for Mac OS X on conda. Travis also builds a version from source, also with BLA_VENDOR="Apple". I don't have a Mac. No idea how hard it is to install MKL (possibly from conda) or OpenBLAS. Homebrew or other tools.

roryyorke commented 4 years ago

New features

Added periodic Schur decomposition functions `mb03vd`, `mb03vy`, and `mb03wd`

Contributed by @bnavigator via pull request #88.

Added `ab08nz`, allowing one to find zeros of complex-valued state-space models

Contributed by @lytex and @bnavigator via pull request #96.

Added `mb03rd`, Schur to block-diagonal transform

Contributed by @bnavigator and @repagh via pull request #116.

Added `sb01fd` H-infinity solver

Contributed by @repagh via #118. Already present sb10ad searches for a minimum gamma value for a given H-infinity problem; sb10fd, by contrast, only attempts to solve for a given gamma value. This can be used to test for admissible gamma values, which could be useful for problems like python-control/python-control#367.

New Slycot exception hierarchy

@bnavigator, with support from @repagh, greatly improved Slycot error handling. Slycot routines now raise SlycotValueError where they would previously have raised ValueError, and SlycotArithmeticError where they would have raised ArithmeticError.
These changes are backwardly compatible: SlycotValueError is a subclasss of ValueError, and SlycotArithmeticError is a subclass of ArithmeticError. As part of this, many of the Slycot function docstrings were changed to conform to numpydoc conventions.

This supersedes an earlier fix for python-control#347 made during 0.4.0 development.

Override XERBLA

In #128, with extra testing in #130, @bnavigator overrode the BLAS error function XERBLA that SLICOT routines use to report errors; the BLAS-provided XERBLA can terminate the whole Python process.

Fixes

Correct application of DGEBAL in TB01TD and TB05AD

@repagh fixed long-standing #11 with PR #122.

Fixed ab01nd for jobz='N' case

@bnavigator contributed this fix in #129.

Build

Contributed by @bnavigator, @repagh, and @rabraker:

conda build recipes no longer use pip
added conda recipes for MKL and Apple
removed conda recipes for Windows; no developer is using them, and they weren't working
general simplifications and improvements of build
the signature files (.pyf files) are now a dependency of wrapper generation

Testing

Contributed by @bnavigator and @lytex:

Tests added for mb05nd, mc01td, ab08nd, ab08nz, mb03vd, mb03vy, mb03wd.
Tests fixed or expanded for mb05md, sg03ad, td04ad.
Switched to pytest; Slycot CI now uses pytest for both the Slycot and python-control test suites.
CI now builds and tests for Linux and macOS. The Linux builds include OpenBLAS and MKL variants.
Examples are run as part of tests. Outputs are not checked, but errors and warnings are.
CI coveralls.io integration fixed.
General test clean-up.
Test files are no longer executable.

Other changes

@bnavigator contributed many other code improvements:

Fix for mb05nd.
Lots of clean-up: removal of unused mathematical.pyf, many docstring fixes, and some PEP 8 conformance changes to Python code.
Fix for Fortran-compiler dependence on machine value used to represent logical values .TRUE. and .FALSE.

@repagh fired up his time machine and ensured punch-card compatibility of the SLICOT Fortran code by limiting line lengths to 72 characters.

Pull requests and issues associated with the release

PRs merged: #71, #72, #75, #78, #91, #93, #79, #82, #83, #84, #87, #88, #96, #97, #101 , #103, #104, #105, #110, #114, #115, #116, #117, #118, #120, #121, #122, #124, #125, #128, #129, #130, #132, #133 Issues fixed: #11, #76, #86, #44, #102, #106, #119, #126 Issues closed ("wont-fix"): #46, #94, #99, #111

bnavigator commented 4 years ago

I would also use 0.4.0. We added some functionality, not only bug-fixes. https://semver.org/

murrayrm commented 4 years ago

Just catching up on this.

Bumping the version number to 0.4.0 sounds fine.

I checked out the branch for PR #101 and tried on my Mac. Building (and installing) via setup.py worked fine, following the list of dependences in PR #101. However, when I tried to run python setup.py test I get a segmentation fault (?). This does not happen with version 0.3.3 of slycot, so seems like something is broken.

This could be something on my end, so let me do some work to make sure that this is reproducible.

bnavigator commented 4 years ago

However, when I tried to run python setup.py test I get a segmentation fault (?). This does not happen with version 0.3.3 of slycot, so seems like something is broken.

I guess it is during a unit test? Can you tell which one? Also, what about version 0.3.5?

murrayrm commented 4 years ago

Confirming that I am able to successfully compile and run tests for slycot-0.3.5 but not for the master branch on MacOS 10.15.4 running Python 3.7 with gcc, openblas, and scikit-build from conda-forge.

The core dump happens if I just run import slycot in Python, so it is not associated with the unit tests directly.

I'm also going to try using the conda build recipe and see if that does anything different.

bnavigator commented 4 years ago

I checked out the branch for PR #101 and tried on my Mac

91, #92, #93 also have some back and forth discussion about getting Mac to build. Turns out it is crucial, that the correct MacOSX SDK is used. Maybe we should also add this to the README.rst in #101.

murrayrm commented 4 years ago

I also tried using conda-recipe-apple and got an error about not being able to find scikit-build. That can be fixed using conda build -c conda-forge condo-recipe-apple, but I still get lots of errors:

conda build -c conda-forge conda-recipe-apple
No numpy version specified in conda_build_config.yaml.  Falling back to default numpy value of 1.11
WARNING:conda_build.metadata:No numpy version specified in conda_build_config.yaml.  Falling back to default numpy value of 1.11
remote: Enumerating objects: 2, done.
remote: Counting objects: 100% (2/2), done.
remote: Compressing objects: 100% (2/2), done.
remote: Total 2 (delta 0), reused 0 (delta 0)
Unpacking objects: 100% (2/2), done.
From /Users/murray/Dropbox/macosx/src/slycot/murrayrm
 * [new ref]                    -> _conda_cache_origin_head
 * [new tag]         v0.3.4     -> v0.3.4
 * [new tag]         v0.3.5     -> v0.3.5
Cloning into '/Users/murray/anaconda3/conda-bld/slycot_1586708649371/work'...
done.
checkout: 'HEAD'
Your branch is up to date with 'origin/_conda_cache_origin_head'.
==> git log -n1 <==

commit ac53a0a744a73450daa23ef7bf8cfae5bcb01e5d
Author: bnavigator <code@bnavigator.de>
Date:   Fri Apr 3 19:25:07 2020 +0200

    switch to pytest for python-control on travis, combine coverage for coveralls

==> git describe --tags --dirty <==

v0.3.5-68-gac53a0a

==> git status <==

On branch _conda_cache_origin_head
Your branch is up to date with 'origin/_conda_cache_origin_head'.

nothing to commit, working tree clean

Adding in variants from internal_defaults
INFO:conda_build.variants:Adding in variants from internal_defaults
Adding in variants from /Users/murray/Dropbox/macosx/src/slycot/murrayrm/conda-recipe-apple/conda_build_config.yaml
INFO:conda_build.variants:Adding in variants from /Users/murray/Dropbox/macosx/src/slycot/murrayrm/conda-recipe-apple/conda_build_config.yaml
Attempting to finalize metadata for slycot
INFO:conda_build.metadata:Attempting to finalize metadata for slycot
Collecting package metadata (repodata.json): ...working... done
Solving environment: ...working... done
Collecting package metadata (repodata.json): ...working... done
Solving environment: ...working... done
Collecting package metadata (repodata.json): ...working... done
Solving environment: ...working... done
Collecting package metadata (repodata.json): ...working... done
Solving environment: ...working... done
BUILD START: ['slycot-0.3.5-py37gac53a0a_mkl_68.tar.bz2']
Collecting package metadata (repodata.json): ...working... done
Solving environment: ...working... done

## Package Plan ##

  environment location: /Users/murray/anaconda3/conda-bld/slycot_1586708649371/_h_env_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_pla

The following NEW packages will be INSTALLED:

    bzip2:           1.0.8-h0b31af3_2           conda-forge
    ca-certificates: 2020.4.5.1-hecc5488_0      conda-forge
    certifi:         2020.4.5.1-py37hc8dfbb8_0  conda-forge
    cmake:           3.17.0-hd28f656_0          conda-forge
    distro:          1.4.0-py_0                 conda-forge
    expat:           2.2.9-h4a8c4bd_2           conda-forge
    krb5:            1.17.1-h1752a42_0          conda-forge
    libblas:         3.8.0-16_openblas          conda-forge
    libcblas:        3.8.0-16_openblas          conda-forge
    libcurl:         7.69.1-hc0b9707_0          conda-forge
    libcxx:          10.0.0-0                   conda-forge
    libedit:         3.1.20170329-hcfe32e1_1001 conda-forge
    libffi:          3.2.1-h4a8c4bd_1007        conda-forge
    libgfortran:     4.0.0-2                    conda-forge
    liblapack:       3.8.0-16_openblas          conda-forge
    libopenblas:     0.3.9-h3d69b6c_0           conda-forge
    libssh2:         1.8.2-hcdc9a53_2           conda-forge
    libuv:           1.34.0-h0b31af3_0          conda-forge
    llvm-openmp:     10.0.0-h28b9765_0          conda-forge
    ncurses:         6.1-h0a44026_1002          conda-forge
    ninja:           1.10.0-ha1b3eb9_0          conda-forge
    numpy:           1.11.3-py37hdf140aa_1207   conda-forge
    openssl:         1.1.1f-h0b31af3_0          conda-forge
    packaging:       20.1-py_0                  conda-forge
    pip:             20.0.2-py_2                conda-forge
    pyparsing:       2.4.7-pyh9f0ad1d_0         conda-forge
    python:          3.7.6-h90870a6_5_cpython   conda-forge
    python_abi:      3.7-1_cp37m                conda-forge
    readline:        8.0-hcfe32e1_0             conda-forge
    rhash:           1.3.6-h1de35cc_1001        conda-forge
    scikit-build:    0.10.0-py37h4a8c4bd_0      conda-forge
    setuptools:      46.1.3-py37hc8dfbb8_0      conda-forge
    six:             1.14.0-py_1                conda-forge
    sqlite:          3.30.1-h93121df_0          conda-forge
    tk:              8.6.10-hbbe82c9_0          conda-forge
    wheel:           0.34.2-py_1                conda-forge
    xz:              5.2.5-h0b31af3_0           conda-forge
    zlib:            1.2.11-h0b31af3_1006       conda-forge

Preparing transaction: ...working... done
Verifying transaction: ...working... done
Executing transaction: ...working... done
Collecting package metadata (repodata.json): ...working... done
Solving environment: ...working... done
Collecting package metadata (repodata.json): ...working... done
Solving environment: ...working... done

## Package Plan ##

  environment location: /Users/murray/anaconda3/conda-bld/slycot_1586708649371/_build_env

The following NEW packages will be INSTALLED:

    cctools:              927.0.2-h5ba7a2e_4       conda-forge
    clang:                9.0.1-default_hf57f61e_0 conda-forge
    clang_osx-64:         9.0.1-h05bbb7f_0         conda-forge
    clangxx:              9.0.1-default_hf57f61e_0 conda-forge
    compiler-rt:          9.0.1-h6a512c6_3         conda-forge
    compiler-rt_osx-64:   9.0.1-h99342c6_3         conda-forge
    gfortran_impl_osx-64: 7.3.0-hf4212f2_2         conda-forge
    gfortran_osx-64:      7.3.0-h22b1bf0_9         conda-forge
    gmp:                  6.2.0-h4a8c4bd_2         conda-forge
    isl:                  0.19-0                   conda-forge
    ld64:                 450.3-h3c32e8a_4         conda-forge
    libcxx:               10.0.0-0                 conda-forge
    libgfortran:          4.0.0-2                  conda-forge
    libiconv:             1.15-h0b31af3_1006       conda-forge
    libllvm9:             9.0.1-ha1b3eb9_0         conda-forge
    llvm-openmp:          10.0.0-h28b9765_0        conda-forge
    mpc:                  1.1.0-h4160ff4_1007      conda-forge
    mpfr:                 4.0.2-h65ac59c_1         conda-forge
    tapi:                 1000.10.8-ha1b3eb9_4     conda-forge
    zlib:                 1.2.11-h0b31af3_1006     conda-forge

Preparing transaction: ...working... done
Verifying transaction: ...working... done
Executing transaction: ...working... done
source tree in: /Users/murray/anaconda3/conda-bld/slycot_1586708649371/work
export PREFIX=/Users/murray/anaconda3/conda-bld/slycot_1586708649371/_h_env_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_pla
export BUILD_PREFIX=/Users/murray/anaconda3/conda-bld/slycot_1586708649371/_build_env
export SRC_DIR=/Users/murray/anaconda3/conda-bld/slycot_1586708649371/work
INFO: activate-gfortran_osx-64.sh made the following environmental changes:
+DEBUG_FFLAGS=-march=nocona -mtune=core2 -ftree-vectorize -fPIC -fstack-protector -O2 -pipe -isystem $PREFIX/include -fdebug-prefix-map=$SRC_DIR=/usr/local/src/conda/slycot-0.3.5 -fdebug-prefix-map=$PREFIX=/usr/local/src/conda-prefix -march=nocona -mtune=core2 -ftree-vectorize -fPIC -fstack-protector -O2 -pipe -Og -g -Wall -Wextra -fcheck=all -fbacktrace -fimplicit-none -fvar-tracking-assignments
+DEBUG_FORTRANFLAGS=-march=nocona -mtune=core2 -ftree-vectorize -fPIC -fstack-protector -O2 -pipe -isystem $PREFIX/include -fdebug-prefix-map=$SRC_DIR=/usr/local/src/conda/slycot-0.3.5 -fdebug-prefix-map=$PREFIX=/usr/local/src/conda-prefix -march=nocona -mtune=core2 -ftree-vectorize -fPIC -fstack-protector -O2 -pipe -Og -g -Wall -Wextra -fcheck=all -fbacktrace -fimplicit-none -fvar-tracking-assignments
+F77=$BUILD_PREFIX/bin/x86_64-apple-darwin13.4.0-gfortran
+F90=$BUILD_PREFIX/bin/x86_64-apple-darwin13.4.0-gfortran
+F95=$BUILD_PREFIX/bin/x86_64-apple-darwin13.4.0-gfortran
+FC=$BUILD_PREFIX/bin/x86_64-apple-darwin13.4.0-gfortran
+FFLAGS=-march=nocona -mtune=core2 -ftree-vectorize -fPIC -fstack-protector -O2 -pipe -isystem $PREFIX/include -fdebug-prefix-map=$SRC_DIR=/usr/local/src/conda/slycot-0.3.5 -fdebug-prefix-map=$PREFIX=/usr/local/src/conda-prefix
+FORTRANFLAGS=-march=nocona -mtune=core2 -ftree-vectorize -fPIC -fstack-protector -O2 -pipe -isystem $PREFIX/include -fdebug-prefix-map=$SRC_DIR=/usr/local/src/conda/slycot-0.3.5 -fdebug-prefix-map=$PREFIX=/usr/local/src/conda-prefix
+GFORTRAN=$BUILD_PREFIX/bin/x86_64-apple-darwin13.4.0-gfortran
+HOST=x86_64-apple-darwin13.4.0
+LDFLAGS= -Wl,-rpath,$PREFIX/lib -L$PREFIX/lib
INFO: activate_clang_osx-64.sh made the following environmental changes:
+AR=$BUILD_PREFIX/bin/x86_64-apple-darwin13.4.0-ar
+AS=$BUILD_PREFIX/bin/x86_64-apple-darwin13.4.0-as
+CC=x86_64-apple-darwin13.4.0-clang
+CFLAGS=-march=core2 -mtune=haswell -mssse3 -ftree-vectorize -fPIC -fPIE -fstack-protector-strong -O2 -pipe -isystem $PREFIX/include -fdebug-prefix-map=$SRC_DIR=/usr/local/src/conda/slycot-0.3.5 -fdebug-prefix-map=$PREFIX=/usr/local/src/conda-prefix
+CHECKSYMS=$BUILD_PREFIX/bin/x86_64-apple-darwin13.4.0-checksyms
+CLANG=$BUILD_PREFIX/bin/x86_64-apple-darwin13.4.0-clang
+CMAKE_PREFIX_PATH=:$PREFIX
+CODESIGN_ALLOCATE=$BUILD_PREFIX/bin/x86_64-apple-darwin13.4.0-codesign_allocate
+CPPFLAGS=-D_FORTIFY_SOURCE=2 -mmacosx-version-min=10.9 -isystem $PREFIX/include
+DEBUG_CFLAGS=-march=core2 -mtune=haswell -mssse3 -ftree-vectorize -fPIC -fPIE -fstack-protector-strong -O2 -pipe -Og -g -Wall -Wextra -isystem $PREFIX/include -fdebug-prefix-map=$SRC_DIR=/usr/local/src/conda/slycot-0.3.5 -fdebug-prefix-map=$PREFIX=/usr/local/src/conda-prefix
+INDR=$BUILD_PREFIX/bin/x86_64-apple-darwin13.4.0-indr
+INSTALL_NAME_TOOL=$BUILD_PREFIX/bin/x86_64-apple-darwin13.4.0-install_name_tool
+LD=$BUILD_PREFIX/bin/x86_64-apple-darwin13.4.0-ld
+LDFLAGS_LD=-pie -headerpad_max_install_names -dead_strip_dylibs -rpath $PREFIX/lib -L$PREFIX/lib
+LIBTOOL=$BUILD_PREFIX/bin/x86_64-apple-darwin13.4.0-libtool
+LIPO=$BUILD_PREFIX/bin/x86_64-apple-darwin13.4.0-lipo
+NM=$BUILD_PREFIX/bin/x86_64-apple-darwin13.4.0-nm
+NMEDIT=$BUILD_PREFIX/bin/x86_64-apple-darwin13.4.0-nmedit
+OTOOL=$BUILD_PREFIX/bin/x86_64-apple-darwin13.4.0-otool
+PAGESTUFF=$BUILD_PREFIX/bin/x86_64-apple-darwin13.4.0-pagestuff
+RANLIB=$BUILD_PREFIX/bin/x86_64-apple-darwin13.4.0-ranlib
+REDO_PREBINDING=$BUILD_PREFIX/bin/x86_64-apple-darwin13.4.0-redo_prebinding
+SEGEDIT=$BUILD_PREFIX/bin/x86_64-apple-darwin13.4.0-segedit
+SEG_ADDR_TABLE=$BUILD_PREFIX/bin/x86_64-apple-darwin13.4.0-seg_addr_table
+SEG_HACK=$BUILD_PREFIX/bin/x86_64-apple-darwin13.4.0-seg_hack
+SIZE=$BUILD_PREFIX/bin/x86_64-apple-darwin13.4.0-size
+STRINGS=$BUILD_PREFIX/bin/x86_64-apple-darwin13.4.0-strings
+STRIP=$BUILD_PREFIX/bin/x86_64-apple-darwin13.4.0-strip
+_CONDA_PYTHON_SYSCONFIGDATA_NAME=_sysconfigdata_x86_64_apple_darwin13_4_0
Not searching for unused variables given on the command line.
CMake Warning at /Users/murray/anaconda3/conda-bld/slycot_1586708649371/_h_env_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_pla/share/cmake-3.17/Modules/Platform/Darwin-Initialize.cmake:221 (message):
  Ignoring CMAKE_OSX_SYSROOT value:

   /opt/MacOSX10.9.sdk

  because the directory does not exist.
Call Stack (most recent call first):
  /Users/murray/anaconda3/conda-bld/slycot_1586708649371/_h_env_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_pla/share/cmake-3.17/Modules/CMakeSystemSpecificInitialize.cmake:21 (include)
  CMakeLists.txt:2 (PROJECT)

-- The C compiler identification is Clang 9.0.1
-- Check for working C compiler: $BUILD_PREFIX/bin/x86_64-apple-darwin13.4.0-clang
-- Check for working C compiler: $BUILD_PREFIX/bin/x86_64-apple-darwin13.4.0-clang - broken
CMake Error at /Users/murray/anaconda3/conda-bld/slycot_1586708649371/_h_env_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_pla/share/cmake-3.17/Modules/CMakeTestCCompiler.cmake:60 (message):
  The C compiler

    "/Users/murray/anaconda3/conda-bld/slycot_1586708649371/_build_env/bin/x86_64-apple-darwin13.4.0-clang"

  is not able to compile a simple test program.

  It fails with the following output:

    Change Dir: /Users/murray/anaconda3/conda-bld/slycot_1586708649371/work/_cmake_test_compile/build/CMakeFiles/CMakeTmp

    Run Build Command(s):/usr/bin/make cmTC_23574/fast && /Applications/Xcode.app/Contents/Developer/usr/bin/make -f CMakeFiles/cmTC_23574.dir/build.make CMakeFiles/cmTC_23574.dir/build
    Building C object CMakeFiles/cmTC_23574.dir/testCCompiler.c.o
    /Users/murray/anaconda3/conda-bld/slycot_1586708649371/_build_env/bin/x86_64-apple-darwin13.4.0-clang   -march=core2 -mtune=haswell -mssse3 -ftree-vectorize -fPIC -fPIE -fstack-protector-strong -O2 -pipe -isystem /Users/murray/anaconda3/conda-bld/slycot_1586708649371/_h_env_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_pla/include -fdebug-prefix-map=/Users/murray/anaconda3/conda-bld/slycot_1586708649371/work=/usr/local/src/conda/slycot-0.3.5 -fdebug-prefix-map=/Users/murray/anaconda3/conda-bld/slycot_1586708649371/_h_env_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_pla=/usr/local/src/conda-prefix -isysroot /opt/MacOSX10.9.sdk  -arch x86_64 -mmacosx-version-min=10.9   -o CMakeFiles/cmTC_23574.dir/testCCompiler.c.o   -c /Users/murray/anaconda3/conda-bld/slycot_1586708649371/work/_cmake_test_compile/build/CMakeFiles/CMakeTmp/testCCompiler.c
    clang-9: warning: no such sysroot directory: '/opt/MacOSX10.9.sdk' [-Wmissing-sysroot]
    Linking C executable cmTC_23574
    /Users/murray/anaconda3/conda-bld/slycot_1586708649371/_h_env_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_pla/bin/cmake -E cmake_link_script CMakeFiles/cmTC_23574.dir/link.txt --verbose=1
    /Users/murray/anaconda3/conda-bld/slycot_1586708649371/_build_env/bin/x86_64-apple-darwin13.4.0-clang -march=core2 -mtune=haswell -mssse3 -ftree-vectorize -fPIC -fPIE -fstack-protector-strong -O2 -pipe -isystem /Users/murray/anaconda3/conda-bld/slycot_1586708649371/_h_env_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_pla/include -fdebug-prefix-map=/Users/murray/anaconda3/conda-bld/slycot_1586708649371/work=/usr/local/src/conda/slycot-0.3.5 -fdebug-prefix-map=/Users/murray/anaconda3/conda-bld/slycot_1586708649371/_h_env_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_pla=/usr/local/src/conda-prefix -isysroot /opt/MacOSX10.9.sdk  -arch x86_64 -mmacosx-version-min=10.9 -Wl,-search_paths_first -Wl,-headerpad_max_install_names -Wl,-rpath,/Users/murray/anaconda3/conda-bld/slycot_1586708649371/_h_env_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_pla/lib -L/Users/murray/anaconda3/conda-bld/slycot_1586708649371/_h_env_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_pla/lib -v -isysroot /opt/MacOSX10.9.sdk  CMakeFiles/cmTC_23574.dir/testCCompiler.c.o  -o cmTC_23574 
    clang version 9.0.1 
    Target: x86_64-apple-darwin13.4.0
    Thread model: posix
    InstalledDir: /Users/murray/anaconda3/conda-bld/slycot_1586708649371/_build_env/bin
    clang-9: warning: no such sysroot directory: '/opt/MacOSX10.9.sdk' [-Wmissing-sysroot]
     "/Users/murray/anaconda3/conda-bld/slycot_1586708649371/_build_env/bin/x86_64-apple-darwin13.4.0-ld" -demangle -lto_library /Users/murray/anaconda3/conda-bld/slycot_1586708649371/_build_env/lib/libLTO.9.dylib -dynamic -arch x86_64 -macosx_version_min 10.9.0 -pie -syslibroot /opt/MacOSX10.9.sdk -o cmTC_23574 -L/Users/murray/anaconda3/conda-bld/slycot_1586708649371/_h_env_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_pla/lib -search_paths_first -headerpad_max_install_names -rpath /Users/murray/anaconda3/conda-bld/slycot_1586708649371/_h_env_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_pla/lib CMakeFiles/cmTC_23574.dir/testCCompiler.c.o -lSystem /Users/murray/anaconda3/conda-bld/slycot_1586708649371/_build_env/lib/clang/9.0.1/lib/darwin/libclang_rt.osx.a
    ld: library not found for -lSystem
    clang-9: error: linker command failed with exit code 1 (use -v to see invocation)
    make[1]: *** [cmTC_23574] Error 1
    make: *** [cmTC_23574/fast] Error 2

  CMake will not be able to correctly generate this project.
Call Stack (most recent call first):
  CMakeLists.txt:3 (ENABLE_LANGUAGE)

-- Configuring incomplete, errors occurred!
See also "$SRC_DIR/_cmake_test_compile/build/CMakeFiles/CMakeOutput.log".
See also "$SRC_DIR/_cmake_test_compile/build/CMakeFiles/CMakeError.log".
********************************************************************************
scikit-build could not get a working generator for your system. Aborting build.

Building MacOSX wheels for Python 3.7 requires XCode.
Get it here:

  https://developer.apple.com/xcode/

********************************************************************************

--------------------------------------------------------------------------------
-- Trying "Unix Makefiles" generator
--------------------------------
---------------------------
----------------------
-----------------
------------
-------
--
--
-------
------------
-----------------
----------------------
---------------------------
--------------------------------
-- Trying "Unix Makefiles" generator - failure
--------------------------------------------------------------------------------

Traceback (most recent call last):
  File "/Users/murray/anaconda3/bin/conda-build", line 11, in <module>
    sys.exit(main())
  File "/Users/murray/anaconda3/lib/python3.7/site-packages/conda_build/cli/main_build.py", line 469, in main
    execute(sys.argv[1:])
  File "/Users/murray/anaconda3/lib/python3.7/site-packages/conda_build/cli/main_build.py", line 460, in execute
    verify=args.verify, variants=args.variants)
  File "/Users/murray/anaconda3/lib/python3.7/site-packages/conda_build/api.py", line 209, in build
    notest=notest, need_source_download=need_source_download, variants=variants)
  File "/Users/murray/anaconda3/lib/python3.7/site-packages/conda_build/build.py", line 2344, in build_tree
    notest=notest,
  File "/Users/murray/anaconda3/lib/python3.7/site-packages/conda_build/build.py", line 1492, in build
    cwd=src_dir, stats=build_stats)
  File "/Users/murray/anaconda3/lib/python3.7/site-packages/conda_build/utils.py", line 398, in check_call_env
    return _func_defaulting_env_to_os_environ('call', *popenargs, **kwargs)
  File "/Users/murray/anaconda3/lib/python3.7/site-packages/conda_build/utils.py", line 378, in _func_defaulting_env_to_os_environ
    raise subprocess.CalledProcessError(proc.returncode, _args)
subprocess.CalledProcessError: Command '['/bin/bash', '-o', 'errexit', '/Users/murray/anaconda3/conda-bld/slycot_1586708649371/work/conda_build.sh']' returned non-zero exit status 1.

bnavigator commented 4 years ago

Yep, looks like it doesn't find the MacOSX SDK. Did you check the travis configuration how it is handled there?

murrayrm commented 4 years ago

Normally (and for the case of v0.3.5), I can just use the standard build process (via setup.py) and things work just like on linux. At the least I think we should get that working properly.

Let me do some debugging and see if I can find out what is causing the core dump. I've never seen that happen before, so may take a while.

I'll also test on MacOS 10.14 (Mojave) and make sure it isn't something having to do with 10.15 (Catalina).

bnavigator commented 4 years ago

Just as a hint for your debugging: I have seen coredumps building Slycot on Linux and the reason was often, that the wrong LAPACK/BLAS implementation was used. I think it has to be the same as Numpy is linked to on the system.

roryyorke commented 4 years ago

I got a build working in Windows -- a "python setup.py install" in a conda environment. However, a unit test fails! See #102 for build script, and failure.

roryyorke commented 4 years ago

Any updates on setup.py on macOS?

It would be nice to get #104 and #105 into the release too, but then we should all be doing our build tests with those merged in.

I think we don't have to have a fix for #106 in the release, since it principally affects CI.

bnavigator commented 4 years ago

The CI builds with setup.py in 3 different macOS environments fine (using BLA_VENDOR="Apple")

roryyorke commented 4 years ago

@bnavigator great, thanks. @murrayrm any luck on your system(s)?

murrayrm commented 4 years ago

I need to check more carefully (eg, make sure I didn't leave anything from the previous build lying around), but a quick rebuild off of the latest commit on the branch for PR #101 generated the same segmentation fault. Let me know if that is not the right thing to be testing...

roryyorke commented 4 years ago

@murrayrm could you rather build latest master (003a2af) ? #101 is a bit behind some recent merges.

roryyorke commented 4 years ago

~~FWIW, on Windows I successfully built 003a2af with Python 3.8, using conda-forge flang and Visual Studio build tools 14.2.~~

This was wrong -- I was looking at an old compilation log file.

Two hours and a sanity point later, and I still can't build with Python 3.8 (well, I can build, but the result fails to import). Below is the build script; log attached.

If I didn't install the libblas=*=*netlib package, CMake couldn't find BLAS (I tried 'plain' libblas, also nomkl libblas). I was hoping to try swapping out netlib for openblas and mkl after the build worked, but obviously never got that far.

setlocal

call "c:\Program Files (x86)\Microsoft Visual Studio\2019\BuildTools\Common7\Tools\VsDevCmd.bat" -arch=amd64

call %HOMEDRIVE%%HOMEPATH%\Miniconda3\Scripts\activate base

set CONDA=conda

CALL %CONDA% --version
@if errorlevel 1 exit /b %ERRORLEVEL%

CALL %CONDA% activate base
@if errorlevel 1 exit /b %ERRORLEVEL%

CALL %CONDA% remove --yes --all --name build-slycot-py38
@if errorlevel 1 exit /b %ERRORLEVEL%

CALL %CONDA% create --quiet --yes --channel conda-forge --name build-slycot-py38 python=3.8 numpy scipy libblas=*=*netlib liblapack=*=*netlib scikit-build flang pytest
@if errorlevel 1 exit /b %ERRORLEVEL%

CALL %CONDA% activate build-slycot-py38
@if errorlevel 1 exit /b %ERRORLEVEL%

cd slycot
@if errorlevel 1 exit /b %ERRORLEVEL%

git describe --always --dirty
@if errorlevel 1 exit /b %ERRORLEVEL%

git status --ignored
@if errorlevel 1 exit /b %ERRORLEVEL%

python setup.py install
@if errorlevel 1 exit /b %ERRORLEVEL%

cd slycot\tests
pytest

py38-compile-log.txt

murrayrm commented 4 years ago

Still getting a segmentation fault on 003a2af with Python 3.7 using conda-forge gcc and Xcode on MacOS 10.15.4 (Catalina). I'll try setting up a new conda test environment and installing from scratch following the directions in the README.

murrayrm commented 4 years ago

Just checked and the exact same conda environment builds v0.3.5 just fine and all tests pass. So it is something in master that is clogging things up.

A diff between current master and v0.3.5 is here. It looks like the issue is probably around the version of OpenBLAS/LAPACK that I am using? Here's what I am using

# packages in environment at /Users/murray/anaconda3/envs/slycot-test:
#
# Name                    Version                   Build  Channel
liblapack                 3.8.0               16_openblas    conda-forge
libopenblas               0.3.9                h3d69b6c_0    conda-forge
openblas                  0.3.9                h2f2564b_0    conda-forge

Hints on things to try?

moorepants commented 4 years ago

It's probably good to update the copyright statement in the license before a new release: https://github.com/python-control/Slycot/blob/master/COPYING

Seems like nothing has been added for the contributions since 2011.

murrayrm commented 4 years ago

Tried with BLA_VENDOR="Apple" when running setup.py and the Apple version of LAPACK still gets picked up, but still segfaults.

What I get in the default setup.py is the following relative to BLAS/LAPACK:

-- Found BLAS: /Users/murray/anaconda3/envs/slycot-test/lib/libopenblas.dylib  
-- Looking for Fortran cheev
-- Looking for Fortran cheev - found
-- A library with LAPACK API found.
-- LAPACK: /Users/murray/anaconda3/envs/slycot-test/lib/libopenblas.dylib
-- Slycot version: 0.3.5.76

bnavigator commented 4 years ago

Could you please attach a full log for both builds? Plain setup.py and conda?

murrayrm commented 4 years ago

Logs attached for four different cases:

v0.3.5 using setup.py (working)
master using setup.py (segfault)
master using conda-recipe-apple (dependency fail)
master using condo-recipe-openblas (same dependence fail)

v0.3.5_setup.txt master_setup.txt master_conda-recipe-apple.txt master_conda-recipe-openblas.txt

roryyorke commented 4 years ago

Windows 10 / Python 3.8 / conda-forge tests pass with python -m unittest, run in slycot/tests; the failed log above was with pytest. I tried python -m pytest too, in case a "pytest" program from some other path was being run, but that gave the same failing result.

So the Windows build is probably OK, but it would be good to get pytest working.

I'll still try swapping out libblas=*=*netlib (and lapack) for openblas and mkl, and see if the tests still pass.

roryyorke commented 4 years ago

The unit tests still pass after installing openblas (libblas=*=*openblas and liblapack=*=*openblas), and mkl (libblas=*=*mkl and liblapack=*=*mkl). For the latter I also uninstalled libopenblas; for this particular case the conda list outputs are attached.

This seems ideal: one can build against LAPACK, but install with OpenBLAS or MKL. Maybe we should try this on macOS too?

I haven't looked at pytest further at all. conda-mkl-env-list.txt

roryyorke commented 4 years ago

When running pytest in the installed slycot.tests directory it passes. If I move slycot/tests to blah/tests, and run pytest tests in directory blah, they also pass. If I move slycot/tests to test (i.e., in root of working tree), then they again fail.

My guess: pytest wants the tests it runs to be arranged as a Python package (as in, directory with __init_.py) and this confuses the importing when tests appears to be a sub-package of slycot.

We can look at fixing this if we get Windows CI going (#108); for now I'll just recommend python -m unittest on Windows in README.rst

roryyorke commented 4 years ago

@murrayrm I don't think this really help, but FWIW here's what I've found lookin at your non-conda build cases.

The differences in the configuration phase that caught my eye:

v0.3.5

-- LAPACK: /Users/murray/anaconda3/envs/slycot-test/lib/libopenblas.dylib
-- Slycot version: 0.3.5.0
-- Found F2PY: /Users/murray/anaconda3/envs/slycot-test/bin/f2py (found version "2") 
-- binary module suffix .cpython-37m-darwin.so
-- Configuring done

master (003a2af)

-- LAPACK: /Users/murray/anaconda3/envs/slycot-test/lib/libopenblas.dylib
-- Slycot version: 0.3.5.76
-- Found F2PY: /Users/murray/anaconda3/envs/slycot-test/bin/f2py (found version "2") 
-- Performing Test Weak Link MODULE -> SHARED (gnu_ld_ignore) - Failed
-- Performing Test Weak Link MODULE -> SHARED (osx_dynamic_lookup) - Failed
-- Performing Test Weak Link MODULE -> SHARED (no_flag) - Failed
_modinit_prefix:PyInit_
-- Configuring done

So the "binary module suffix" message has disappeared (see diff below - it was our message), and some "weak link" tests have been performed and failed (googling that message doesn't reveal much of help.)

`slycot/CMakeLists.txt` differences

In slycot/CMakeLists.txt, ${SLYCOT_MODULE} was a SHARED libray, on 003a2af it's a MODULE, and is also passed to CMake function (or macro) python_extension_module; I presume that explains some of the differences above.

The other diffs to slycot/CMakeLists.txt seem less important; they're below, except the very large diff which looks like a rearrangement of the SLICOT Fortran source file list.

--- a/slycot/CMakeLists.txt
+++ b/slycot/CMakeLists.txt
@@ -104,5 +109,9 @@

 set(F2PYSOURCE src/_wrapper.pyf)
+set(F2PYSOURCE_DEPS
+  src/analysis.pyf src/math.pyf
+  src/transform.pyf src/synthesis.pyf
+  src/_helper.pyf)

 configure_file(version.py.in version.py @ONLY)

@@ -112,6 +121,8 @@ set(PYSOURCE
   transform.py ${CMAKE_CURRENT_BINARY_DIR}/version.py)

 set(SLYCOT_MODULE "_wrapper")
+find_package(PythonExtensions REQUIRED)
+
 set(GENERATED_MODULE
   ${CMAKE_CURRENT_BINARY_DIR}/${SLYCOT_MODULE}${PYTHON_EXTENSION_MODULE_SUFFIX})

@@ -123,36 +134,22 @@ add_custom_command(
   OUTPUT SLYCOTmodule.c _wrappermodule.c _wrapper-f2pywrappers.f
   COMMAND ${F2PY_EXECUTABLE} -m SLYCOT
   ${CMAKE_CURRENT_SOURCE_DIR}/${F2PYSOURCE}
-  )
+  DEPENDS ${F2PYSOURCE_DEPS} ${F2PYSOURCE}
+)

 add_library(
-  ${SLYCOT_MODULE} SHARED
+  ${SLYCOT_MODULE} MODULE
   SLYCOTmodule.c _wrappermodule.c _wrapper-f2pywrappers.f
   "${PYTHON_SITE}/numpy/f2py/src/fortranobject.c"
   ${FSOURCES})

-set(CMAKE_SHARED_LIBRARY_PREFIX "")
-if (WIN32)
-  set(CMAKE_SHARED_LIBRARY_SUFFIX ".pyd")
-endif()
-set_target_properties(${SLYCOT_MODULE} PROPERTIES
-  OUTPUT_NAME "_wrapper")
-if (WIN32)
-  target_link_libraries(${SLYCOT_MODULE} PUBLIC
-    ${PYTHON_LIBRARIES} ${LAPACK_LIBRARIES}) 
-endif()
+target_link_libraries(${SLYCOT_MODULE}
+  ${LAPACK_LIBRARIES})

 if (UNIX)
-  target_link_libraries(${SLYCOT_MODULE} PUBLIC
-    ${LAPACK_LIBRARIES})
-
   if (APPLE)
     set_target_properties(${SLYCOT_MODULE} PROPERTIES
       LINK_FLAGS  '-Wl,-dylib,-undefined,dynamic_lookup')
-    string(REGEX REPLACE "^([0-9]+)\.([0-9]+)\.[0-9]+$" "\\1\\2"
-      PYMAJORMINOR ${PYTHON_VERSION_STRING})
-    set(CMAKE_SHARED_LIBRARY_SUFFIX ".cpython-${PYMAJORMINOR}m-darwin.so")
-    message(STATUS "binary module suffix ${CMAKE_SHARED_LIBRARY_SUFFIX}")
   else()
     set_target_properties(${SLYCOT_MODULE} PROPERTIES
       LINK_FLAGS  '-Wl,--allow-shlib-undefined')
@@ -166,6 +163,7 @@ target_include_directories(
   ${PYTHON_INCLUDE_DIRS}
   )

+python_extension_module(${SLYCOT_MODULE})
 install(TARGETS ${SLYCOT_MODULE} DESTINATION slycot)
 install(FILES ${PYSOURCE} DESTINATION slycot)

bnavigator commented 4 years ago

@murrayrm, ~why is f2py installed again right before the test run?~ Edit: seems to be a setuptools thing for envs and alternate prefixes

-- Found F2PY: /Users/murray/anaconda3/envs/slycot-test/bin/f2py (found version "2"

[...]

Installed /Users/murray/Dropbox/macosx/src/slycot/murrayrm
Processing dependencies for slycot==0.3.5.0
Searching for numpy==1.18.1
Best match: numpy 1.18.1
Adding numpy 1.18.1 to easy-install.pth file
Installing f2py script to /Users/murray/anaconda3/envs/slycot-test/bin
Installing f2py3 script to /Users/murray/anaconda3/envs/slycot-test/bin
Installing f2py3.7 script to /Users/murray/anaconda3/envs/slycot-test/bin

Using /Users/murray/anaconda3/envs/slycot-test/lib/python3.7/site-packages
Finished processing dependencies for slycot==0.3.5.0
WARNING: Testing via this command is deprecated and will be removed in a future version. Users looking for a generic test entry point independent of test runner are encouraged to use tox.
running build_ext
Running from numpy source directory.
test1_sb10jd (slycot.tests.test_sb10jd.test_sb10jd)
verify the output of sb10jd for a descriptor system ... ok

Are you sure you are using the right version at compile time? That was a problem a few months back when I created the OpenSUSE package: #81.

bnavigator commented 4 years ago

(slycot-test) murray@limani murrayrm % conda build conda-recipe-apple

[...]

conda.exceptions.ResolvePackageNotFound: 
  - scikit-build[version='>=0.10.0']

You forgot to activate the conda-forge channel. See #101 instructions: conda build -c conda-forge conda-recipe-apple

bnavigator commented 4 years ago

@roryyorke

E   ImportError: cannot import name '_wrapper' from partially initialized module 'slycot'

When running pytest in the installed slycot.tests directory it passes. If I move slycot/tests to blah/tests, and run pytest tests in directory blah, they also pass. If I move slycot/tests to test (i.e., in root of working tree), then they again fail.

My guess: pytest wants the tests it runs to be arranged as a Python package (as in, directory with __init_.py) and this confuses the importing when tests appears to be a sub-package of slycot.

We can look at fixing this if we get Windows CI going (#108); for now I'll just recommend python -m unittest on Windows in README.rst

This is not Windows specific. I also get this on Linux when the installed module does not match the module in $srcdir/slycot. Something with the scikit-build or setuptools initiated moving around of the compiled module between _skbuild, installed site-packages and $srcdir/slycot is broken and exposed by the way pytest imports modules.

bnavigator commented 4 years ago

Falling back to old unittest instead of pytest fails to discover a few tests (beside the examples) that are not methods of unitttest classes:

[ben@voyagerS9:~/src/Slycot]% pytest -v --ignore _skbuild                                                                                                    [2]
====================================================================== test session starts ======================================================================
platform linux -- Python 3.8.2, pytest-5.4.1, py-1.8.1, pluggy-0.13.1 -- /usr/bin/python
cachedir: .pytest_cache
rootdir: /home/ben/src/Slycot
plugins: cov-2.8.1, shutil-1.7.0, virtualenv-1.7.0, mock-2.0.0
collected 47 items                                                                                                                                              

slycot/tests/test_ab08n.py::test_ab08n::test_ab08nd PASSED                                                                                                [  2%]
slycot/tests/test_ab08n.py::test_ab08n::test_ab08nz PASSED                                                                                                [  4%]
slycot/tests/test_ag08bd.py::test_tg01fd::test1_ag08bd PASSED                                                                                             [  6%]
slycot/tests/test_ag08bd.py::test_tg01fd::test2_ag08bd PASSED                                                                                             [  8%]
slycot/tests/test_ag08bd.py::test_tg01fd::test3_ag08bd PASSED                                                                                             [ 10%]
slycot/tests/test_ag08bd.py::test_tg01fd::test4_ag08bd PASSED                                                                                             [ 12%]
slycot/tests/test_examples.py::test_example[ab08nd_example] PASSED                                                                                        [ 14%]
slycot/tests/test_examples.py::test_example[ab13bd_example] PASSED                                                                                        [ 17%]
slycot/tests/test_examples.py::test_example[ab13dd_example] PASSED                                                                                        [ 19%]
slycot/tests/test_examples.py::test_example[ab13ed_example] PASSED                                                                                        [ 21%]
slycot/tests/test_examples.py::test_example[ab13fd_example] PASSED                                                                                        [ 23%]
slycot/tests/test_examples.py::test_example[mc01td_example] PASSED                                                                                        [ 25%]
slycot/tests/test_examples.py::test_example[sb02md_example] PASSED                                                                                        [ 27%]
slycot/tests/test_examples.py::test_example[sb02od_example] PASSED                                                                                        [ 29%]
slycot/tests/test_examples.py::test_example[sb03md_example] PASSED                                                                                        [ 31%]
slycot/tests/test_examples.py::test_example[tb01pd_example] PASSED                                                                                        [ 34%]
slycot/tests/test_examples.py::test_example[tb03ad_example] PASSED                                                                                        [ 36%]
slycot/tests/test_examples.py::test_example[tb05ad_example] PASSED                                                                                        [ 38%]
slycot/tests/test_examples.py::test_example[tc04ad_example] PASSED                                                                                        [ 40%]
slycot/tests/test_mb.py::test_mb::test_mb03vd_mb03vy_ex PASSED                                                                                            [ 42%]
slycot/tests/test_mb.py::test_mb::test_mb03wd_ex PASSED                                                                                                   [ 44%]
slycot/tests/test_mb.py::test_mb::test_mb05md PASSED                                                                                                      [ 46%]
slycot/tests/test_mb.py::test_mb::test_mb05nd PASSED                                                                                                      [ 48%]
slycot/tests/test_mc.py::test_mc::test_mc01td PASSED                                                                                                      [ 51%]
slycot/tests/test_mc.py::test_mc::test_mc01td_D PASSED                                                                                                    [ 53%]
slycot/tests/test_mc.py::test_mc::test_mc01td_warnings PASSED                                                                                             [ 55%]
slycot/tests/test_sb.py::test_sb02mt PASSED                                                                                                               [ 57%]
slycot/tests/test_sb.py::test_sb10ad PASSED                                                                                                               [ 59%]
slycot/tests/test_sb.py::test_sb10jd PASSED                                                                                                               [ 61%]
slycot/tests/test_sg02ad.py::test_sg02ad::test_sg02ad_case1 PASSED                                                                                        [ 63%]
slycot/tests/test_sg03ad.py::test_sg03ad::test_sg03ad_b1 PASSED                                                                                           [ 65%]
slycot/tests/test_sg03ad.py::test_sg03ad::test_sg03ad_ex1c PASSED                                                                                         [ 68%]
slycot/tests/test_sg03ad.py::test_sg03ad::test_sg03ad_ex1d PASSED                                                                                         [ 70%]
slycot/tests/test_tb05ad.py::test_tb05ad::test_tb05ad_ag_failure XFAIL                                                                                    [ 72%]
slycot/tests/test_tb05ad.py::test_tb05ad::test_tb05ad_errors PASSED                                                                                       [ 74%]
slycot/tests/test_tb05ad.py::test_tb05ad::test_tb05ad_ng PASSED                                                                                           [ 76%]
slycot/tests/test_tb05ad.py::test_tb05ad::test_tb05ad_nh PASSED                                                                                           [ 78%]
slycot/tests/test_td04ad.py::TestTf2SS::test_mixfeedthrough PASSED                                                                                        [ 80%]
slycot/tests/test_td04ad.py::TestTf2SS::test_staticgain PASSED                                                                                            [ 82%]
slycot/tests/test_td04ad.py::TestTf2SS::test_td04ad_c PASSED                                                                                              [ 85%]
slycot/tests/test_td04ad.py::TestTf2SS::test_td04ad_r PASSED                                                                                              [ 87%]
slycot/tests/test_td04ad.py::TestTf2SS::test_td04ad_static PASSED                                                                                         [ 89%]
slycot/tests/test_td04ad.py::TestTf2SS::test_tfm2ss_6 PASSED                                                                                              [ 91%]
slycot/tests/test_td04ad.py::TestTf2SS::test_toandfrom PASSED                                                                                             [ 93%]
slycot/tests/test_tg01ad.py::test_tg01ad::test1_tg01ad PASSED                                                                                             [ 95%]
slycot/tests/test_tg01fd.py::test_tg01fd::test1_tg01fd PASSED                                                                                             [ 97%]
slycot/tests/test_tg01fd.py::test_tg01fd::test2_tg01fd PASSED                                                                                             [100%]

================================================================= 46 passed, 1 xfailed in 1.73s =================================================================
[ben@voyagerS9:~/src/Slycot]% python -m unittest -v                                                                                                          [0]
test_ab08nd (slycot.tests.test_ab08n.test_ab08n)
Test Construct regular pencil for real matrices ... ok
test_ab08nz (slycot.tests.test_ab08n.test_ab08n)
Test Construct regular pencil for (pseudo) complex matrices ... ok
test1_ag08bd (slycot.tests.test_ag08bd.test_tg01fd)
test [A-lambda*E] ... ok
test2_ag08bd (slycot.tests.test_ag08bd.test_tg01fd)
test [A-lambda*E;C] ... ok
test3_ag08bd (slycot.tests.test_ag08bd.test_tg01fd)
test [A-lambda*E,B] ... ok
test4_ag08bd (slycot.tests.test_ag08bd.test_tg01fd)
test [A-lambda*E,B;C,D] ... ok
test_mb03vd_mb03vy_ex (slycot.tests.test_mb.test_mb)
Test MB03VD and MB03VY ... ok
test_mb03wd_ex (slycot.tests.test_mb.test_mb)
Test MB03WD with the example given in the SLICOT documentation ... ok
test_mb05md (slycot.tests.test_mb.test_mb)
test_mb05md: verify Matrix exponential with slicot doc example ... ok
test_mb05nd (slycot.tests.test_mb.test_mb)
test_mb05nd: verify Matrix exponential and integral ... ok
test_mc01td (slycot.tests.test_mc.test_mc)
test_mc01td: doc example ... ok
test_mc01td_D (slycot.tests.test_mc.test_mc)
test_mc01td_D: test discrete option ... ok
test_mc01td_warnings (slycot.tests.test_mc.test_mc)
test_mc01td_warnings: Test warnings ... ok
test_sg02ad_case1 (slycot.tests.test_sg02ad.test_sg02ad) ... ok
test_sg03ad_b1 (slycot.tests.test_sg03ad.test_sg03ad)
SLICOT doc example / Penzl B.1 ... ok
test_sg03ad_ex1c (slycot.tests.test_sg03ad.test_sg03ad)
Example 1 continuous case ... ok
test_sg03ad_ex1d (slycot.tests.test_sg03ad.test_sg03ad)
Example 1 discrete case ... ok
test_tb05ad_ag_failure (slycot.tests.test_tb05ad.test_tb05ad)
Test tb05ad and job 'AG' (i.e., balancing enabled) fails ... expected failure
test_tb05ad_errors (slycot.tests.test_tb05ad.test_tb05ad)
Test tb05ad error handling. We give wrong inputs and ... ok
test_tb05ad_ng (slycot.tests.test_tb05ad.test_tb05ad)
Test that tb05ad with job 'NG' computes the correct ... ok
test_tb05ad_nh (slycot.tests.test_tb05ad.test_tb05ad)
Test that tb05ad with job = 'NH' computes the correct ... ok
test_mixfeedthrough (slycot.tests.test_td04ad.TestTf2SS)
Test case popping up from control testing ... ok
test_staticgain (slycot.tests.test_td04ad.TestTf2SS)
td04ad: Convert a transferfunction to SS with only static gain ... ok
test_td04ad_c (slycot.tests.test_td04ad.TestTf2SS)
td04ad: Convert with 'C' option ... ok
test_td04ad_r (slycot.tests.test_td04ad.TestTf2SS)
td04ad: Convert with 'R' option ... ok
test_td04ad_static (slycot.tests.test_td04ad.TestTf2SS)
Regression: td04ad (TFM -> SS transformation) for static TFM ... ok
test_tfm2ss_6 (slycot.tests.test_td04ad.TestTf2SS)
Python version of Fortran test program from ... ok
test_toandfrom (slycot.tests.test_td04ad.TestTf2SS) ... ok
test1_tg01ad (slycot.tests.test_tg01ad.test_tg01ad) ... ok
test1_tg01fd (slycot.tests.test_tg01fd.test_tg01fd)
test1: Verify from tg01fd with input parameters according to test in documentation ... ok
test2_tg01fd (slycot.tests.test_tg01fd.test_tg01fd)
verify that Q and Z output with compq and compz set to 'U' equals the dot product of Q and Z input and Q and Z output with compq and compz set to 'I' ... ok

----------------------------------------------------------------------
Ran 31 tests in 0.941s

OK (expected failures=1)

roryyorke commented 4 years ago

Thanks for the note about missing tests. So we must fix this, but can it be deferred to after release?

One fix is to move $srcdir/slycot/tests to $srcdir/test/tests - then pytest runs fine

roryyorke commented 4 years ago

It's probably good to update the copyright statement in the license before a new release: https://github.com/python-control/Slycot/blob/master/COPYING

Seems like nothing has been added for the contributions since 2011.

The CREDITS file is also woefully out of date.

git shortlog -sn is apparently a way to discover authors; from this, I get:

    70  Rene van Paassen
    49  Rory Yorke
    32  Benjamin Greiner
    32  avventi
    30  Enrico Avventi
    28  Richard Murray
    21  James Goppert
    18  bnavigator
    13  Marcus Liljedahl
    13  johannes-scharlach
     8  Scott C. Livingston
     6  Gilles Plessis
     6  Jason K. Moore
     5  Johannes Scharlach
     5  clementm
     3  Ben
     3  eph
     2  Clancy Rowley
     2  JKP3nt
     2  Jerker Nordh
     2  Joris Geysens
     2  René van Paassen
     2  arnold
     2  lytex
     1  ArmstrongJ
     1  Enrico
     1  Jake Vanderplas
     1  Lucas Mehl
     1  root

moorepants commented 4 years ago

I don't have much experience with this; can we add a line Copyright (C) 2012-2020 Slycot team, or do we need individual attribution?

This is sufficient, but it might then be worth including an AUTHORS file that has the shortlog results. The first line can be "The following authors are the Slycot Team".

bnavigator commented 4 years ago

Thanks for the note about missing tests. So we must fix this, but can it be deferred to after release?

It's your decision. I very much prefer having one single instruction "run pytest" instead of telling Windows users to use a different command that does not even do the same thing.

One fix is to move $srcdir/slycot/tests to $srcdir/test/tests - then pytest runs fine

That creates the danger of accidentally installing a test package into site-packages. I would rather keep the tests submodule under slycot

python-control / Slycot

Time to release a new version? #98

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

New features

Added periodic Schur decomposition functions `mb03vd`, `mb03vy`, and `mb03wd`

Added `ab08nz`, allowing one to find zeros of complex-valued state-space models

Added `mb03rd`, Schur to block-diagonal transform

Added `sb01fd` H-infinity solver

New Slycot exception hierarchy

Override XERBLA

Fixes

Correct application of DGEBAL in TB01TD and TB05AD

Fixed ab01nd for jobz='N' case

Build

Testing

Other changes

Pull requests and issues associated with the release

91, #92, #93 also have some back and forth discussion about getting Mac to build. Turns out it is crucial, that the correct MacOSX SDK is used. Maybe we should also add this to the README.rst in #101.

v0.3.5

master (003a2af)

`slycot/CMakeLists.txt` differences

python-control / Slycot

Time to release a new version? #98

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

New features

Added periodic Schur decomposition functions mb03vd, mb03vy, and mb03wd

Added ab08nz, allowing one to find zeros of complex-valued state-space models

Added mb03rd, Schur to block-diagonal transform

Added sb01fd H-infinity solver

New Slycot exception hierarchy

Override XERBLA

Fixes

Correct application of DGEBAL in TB01TD and TB05AD

Fixed ab01nd for jobz='N' case

Build

Testing

Other changes

Pull requests and issues associated with the release

91, #92, #93 also have some back and forth discussion about getting Mac to build. Turns out it is crucial, that the correct MacOSX SDK is used. Maybe we should also add this to the README.rst in #101.

v0.3.5

master (003a2af)

slycot/CMakeLists.txt differences

Added periodic Schur decomposition functions `mb03vd`, `mb03vy`, and `mb03wd`

Added `ab08nz`, allowing one to find zeros of complex-valued state-space models

Added `mb03rd`, Schur to block-diagonal transform

Added `sb01fd` H-infinity solver

`slycot/CMakeLists.txt` differences