Update CI to allow newer Python versions

[eddiebergman]

(WIP) Example of the output for the (github workflows) here:

• Tests
• Wheels

TODOS

• I'm used to using pytest and coverage and having codecov be the CI part that reports back on the PR. I'm not sure what to do here in this case. codecov is free for open-source repos though. Alternatively, a plain text version of the report could be printed and also left as a bot comment on the PR (if it's a PR).
• GraKeL seems to encounter its first backwards breaking change with Python 3.10, namely imports from collections were deprecated a while back and are now available in the abc.collections or typing module of python. See this deprecation warning below the table in the Python3.9 docs.
• Some slight fixes to do with the different builds, just a step by step debug and fix process
• The where=grakel/tests in setup.cfg seems to confuse nosetest when run on windows, trying some workaround or even just let auto-discovery take charge :'(

Changes

• Drop Travis and AppVeyor, instead use Github Actions and cibuildwheel to support linux/mac/windows.
• Unfortunatly, it seems cibuildwheel is relatively indate with pythons end of life schedule. This means that the "easy" setup means new releases will not have builds for 2.7, 3.5, 3.6.
  • Users of these versions running pip install grakel will still recieve a pre-built wheel before this change.
  • As the library is not in active development, this is unlikely to be a problem?
• Moved a lot of the static information from setup.py and setup.cfg to pyproject.toml. *This is a "modern" way to specify meta information about a project, see PEP 621 and setuptools doc page on it. However, this requires setuptools>61.0.0 which is only available for python>=3.6

---

Brief note:

This point above was a bit unnecessary but modern python env/packaging tools such as poetry and flit use this to quickly generate validate dependancy graphs and find valid ones, using only this meta-data from PyPI. This wasn't possible with setup.py specifying requirements as it's a python file and highly un-uniform to parse, while requirements.txt does not handle test/dev dependencies. Specifying them in setup.cfg would work but it provides little benefit.

In general, the python eco-system is heading towards static build/meta/tooling information in pyproject.toml, with python even including a toml parser in it's core library soon. It's probably the safest long term bet at the cost of backwards compatibility.

---

[ysig]

Hi @eddiebergman that's great!

Let me address the above points (Todos -> T, Changes -> C):

T1,2: nosetests seems to be the one that breaks because of callable, see: https://github.com/eddiebergman/GraKeL/actions/runs/3041633008/jobs/4898960711

I have no objection in migrating to pytests.

T4: Can we replace nosetests with pytest or it is a very big deal? (again nosetests was the one used by sklearn at the time)

C2: that's totally fine. As the library is stable for now, we would like to be up to date with python's life-cycle.

C3: That's great!

[ysig]

tldr; I think nosetests is the main bottleneck right now. Let's replace it with whatever you think is best (pytest?): I have no objection to migrate to the proposed "T1" testing pipeline.

[ysig]

@eddiebergman All tests can be found here: https://github.com/ysig/GraKeL/tree/master/grakel/tests

[eddiebergman]

@ysig yup I found them, dont worry :) I ran pytest and nosetest side by side and got the same tests collected and all passed. However pytest additionally reported there's 909 warnings, mostly deprecation warnings from numpy. I can quickly fix those if you like?

[ysig]

@eddiebergman

yup I found them, dont worry :) I ran pytest and nosetest side by side and got the same tests collected and all passed. However pytest additionally reported there's 909 warnings, mostly deprecation warnings from numpy. I can quickly fix those if you like?

Yes that would be great!

[ysig]

So what I see as two main errors for now: 1) https://github.com/ysig/GraKeL/blob/8500dbf6812116459df308396716b7bcae13ef9c/grakel/tools.py#L175 collections.Hashable should probably be replaced by something that is valid for this version of python.

2) Windows errors seem to be related with cvxopt version - not sure how to approach this for now. (why is the output truncated btw?)

[eddiebergman]

Hi @ysig,

Thanks for going in and fixing them :) I'm a bit busy today to do much here but I will fix the wheel builder tests tomorrow or friday!

Best, Eddi

[ysig]

Hi @eddiebergman,

all are passing right now except 2 windows processes:

• I installed python 3.10 on my windows 10 - just with pip install -e ".[dev] - but I don't get the same cvxopt related bug so I'm really confused (all tests pass).

Also for codecov reply to my mail so we configure it. Also can you tell me how I can setup actions to run on my account instead of yours?

For the wheels I see some bugs related with openblas or import k_ij_triangular which is a synonym for the Cython extension not bring installed correctly - not sure why. It is manually setup using:

python setup.py build_ext
python setup.py install

Best, y

[eddiebergman]

Just an update, I reran all the tests: https://github.com/eddiebergman/GraKeL/actions/runs/3051878668

• Seems the wheels are failing due to some import, I will be able to recreate this locally and debug this.

EDIT: Realised the following isn't your code as inside cvxopt instead

• The two failing windows builds seem to be assertion errors and not cvxopt issues from what I see.

          try: f(x, dy, s)
          except ArithmeticError:
  >               raise ValueError("Rank(A) < p or Rank([G; A]) < n")
  E               ValueError: Rank(A) < p or Rank([G; A]) < n

I imagine this could be due to two cases: 1) A flaky test where either the code is not fully correct, the test is wrong or numerical instability 2) Windows 3.9/3.10 ships with a different numerical backend.

I reran the tests, if the same 2 fail then it's very likely 2, otherwise it is case 1.

If it's case 1) then either the test/code has to change, if it's case 2) we need to further debug this in an environment where we can't reproduce it. I think in this case we need to raise the original exception and not throw your own.

The default way to do this with Python 3+ is exception chaining

try:
    f(x, dy, s)
except ArithmeticError as e:
   raise ValueError("Rank(A) < p or Rank([G; A]) < n") from e

However, for the backwards compatibility (if you care for newer versions still being backwards compatible), then I suggest something like:

import traceback

try:
    f(x, dy, s)
except ArithmeticError:
   tb = traceback.format_exc()  # Get str version of current traceback
   raise ValueError("\n" + tb + "\n" + Rank(A) < p or Rank([G; A]) < n")

Best, Eddie

[eddiebergman]

I reran the tests and this time it failed for all windows versions, 3.7 to 3.10. My guess is a slight numerical instability or api change with some library on windows which is causing the ArithmeticError which is then swallod. I will change it to the second proposed error message to gain some more info.

Failing tests in all windows version seem to do with lovasz, whatever that may be.

FAILED grakel/tests/test_Kernel.py::test_lovasz_theta - AssertionError: Value...
FAILED grakel/tests/test_Kernel.py::test_lovasz_theta_pd - AssertionError: Va...
FAILED grakel/tests/test_common.py::test_lovasz_theta - ValueError: Rank(A) <...
FAILED grakel/tests/test_kernels.py::test_lovasz_theta - ValueError: Rank(A) ..

[eddiebergman]

Some more digging and essentially the error is coming from cvxopt as you said, specifically this line.

Something is going wrong with the sdp solver and its inputs.

Some googling of the errors makes me thing there is either an issue in the test case or the implementation itself that was ignored previously but fixed in a more recent version of cvxopt. Looking down the chain all of test failures, it seems to be coming down to some call to what cvxopt call kktsolver which eventually goes to calling lapack functions. Looking at the docs, it gives an example for all the reasons an ArithmeticError might show up (mostly for singular matrices, not full rank or if not semi-positive definite. I imagine these are already transformed matrices and out of your control.

• User reported no issue in Python 2.7 but started in 3.6.5
• Solution on google forums by removing redundant constraints.
• Some testing to see if constraints are actually valid (stackoverflow)

Assuming cvxopt is correct in its code, I would imagine somehow the sdp solver in def _calculate_lovaz_embeddings is given some problem it can't solve or is mis-specified.

Seeing as these are randomly generated problems by your generate_dataset in the test functions, I could see this being an issue occurring. Now why it happens sproadically and only on windows, I have a guess it's to do with the fact test ordering is not entirely deterministic and somehow on windows, this test is more sensitive to the randomized input.

I'll do some brief testing to confirm this.

[eddiebergman]

Going to run 100 different seeds on Linux and see if I can reproduce the error, if not, I will submit the same to github actions and see if we get stochastic failures across seeds or stochastic errors across github workers. If it's stochastic across seeds then somehow there is numerical instability with some windows library. If it's stochastic with respect to github action workers then I imagine there is some variance in terms of the packages they download, which might give a further hint.

    # Trying 100 different seeds
    @pytest.mark.parametrize("r", list(range(100)))
    def test_lovasz_theta(r):
        """Random input test for the Lovasz-theta distance kernel."""
        train, test = generate_dataset(n_graphs=50,
                                       r_vertices=(5, 10),
                                       r_connectivity=(0.4, 0.8),
                                       r_weight_edges=(1, 1),
                                       n_graphs_test=20,
                                       random_state=r,
                                       features=None)

        lt_kernel = LovaszTheta(verbose=verbose, normalize=normalize)

        try:
            lt_kernel.fit_transform(train)
            lt_kernel.transform(test)
            assert True
        except Exception as exception:
            assert False, exception

Update, I also realized that the LovaszTheta class takes a random_state but non is passed during the construction in the test, only the dataset construction, this is another source of test stochasticness.

[eddiebergman]

Seems 100 seeds on Linux still passed everything. From this I'm concluding that it's some numerical instability with Windows that's the prime cause.

Everything stems from _calculate_lovasz_embeddings and fails at this line on Windows. I'm going to essentially just print what goes in and see if we can create a minimal working example that at the very least can be posted to cvxopt if we can not reproduce it.

def _calculate_lovasz_embeddings_(A):
    """Calculate the lovasz embeddings for the given graph.
    Parameters
    ----------
    A : np.array, ndim=2
        The adjacency matrix.
    Returns
    -------
    lovasz_theta : float
        Returns the lovasz theta number.
    optimization_solution : np.array
        Returns the slack variable solution
        of the primal optimization program.
    """
    if A.shape[0] == 1:
        return 1.0
    else:
        tf_adjm = (np.abs(A) <= min_weight)
        np.fill_diagonal(tf_adjm, False)
        i_list, j_list = np.nonzero(np.triu(tf_adjm.astype(int), k=1))

        nv = A.shape[0]
        ne = len(i_list)

        x_list = list()
        e_list = list()
        for (e, (i, j)) in enumerate(zip(i_list, j_list)):
            e_list.append(int(e)), x_list.append(int(i*nv+j))
            if tf_adjm[i, j]:
                e_list.append(int(e)), x_list.append(int(i*nv+j))

    # Add on the last row, diagonal elements
    e_list = e_list+(nv*[ne])
    x_list = x_list+[int(i*nv + i) for i in range(nv)]

    # initialise g sparse (to values -1, based on two list that
    # define index and one that defines shape
    g_sparse = spmatrix(-1, x_list, e_list, (nv*nv, ne+1))

    # Initialise optimization parameters
    h = matrix(-1.0, (nv, nv))
    c = matrix([0.0]*ne + [1.0])

    # Solve the convex optimization problem

    # DEBUGGING > tmp.txt
    # =====================
    print(f"{nv}, {ne}, {e_list}, {x_list}")
    # =====================

    sol = sdp(c, Gs=[g_sparse], hs=[h])

    return np.array(sol['ss'][0]), sol['x'][ne]

Test file:

# test debug
import ...

@pytest.mark.parametrize("nv, ne, e_list, x_list", [...])
def test_debug(nv, ne, e_list, x_list):

    # Taken from the above function
    g_sparse = spmatrix(-1, x_list, e_list, (nv*nv, ne+1))
    h = matrix(-1.0, (nv, nv))
    c = matrix([0.0]*ne + [1.0])

    # Should raise here on windows
    sol = sdp(c, Gs=[g_sparse], hs=[h])
    assert sol is not None

[eddiebergman]

Hurray success! The minimal example works and essentially there are failing cases for the sdp solver only occuring on windows. https://github.com/eddiebergman/GraKeL/actions/runs/3183531279/jobs/5190864022

[eddiebergman]

More debugging: Both cvxopt on windows, linux and mac have installed the same versions of everything except windows seems to install colorama-0.4.5 as an extra dependancy. My guess this has nothing to do with the issue.

[eddiebergman]

I can get it down to one single example that fails on Windows but success and mac/linux. Unfortunatly, I don't really know how to proceed from here due to my lack of knowledge of what is being represented and whether this should be solvable. With your go ahead, I would raise an issue with cvxopt github.

# Passed on linux, mac - Fails on Windows (cvxopt 1.3.0)
import pytest
from cvxopt.base import matrix, spmatrix
from cvxopt.solvers import sdp

@pytest.mark.parametrize(
    "nv, ne, e_list, x_list",
    [
        (9,
         15,
         [ 0, 0, 1, 1, 2, 2, 3, 3, 4, 4, 5, 5, 6, 6, 7, 7, 8, 8, 9, 9, 10, 10, 11, 11, 12, 12, 13, 13, 14, 14, 15, 15, 15, 15, 15, 15, 15, 15, 15],
         [ 2, 2, 3, 3, 6, 6, 7, 7, 12, 12, 14, 14, 15, 15, 21, 21, 22, 22, 23, 23, 25, 25, 41, 41, 44, 44, 53, 53, 71, 71, 0, 10, 20, 30, 40, 50, 60, 70, 80],
        )
    ],
)
def test_windows_sdp(nv, ne, e_list, x_list) -> None:
    # initialise g sparse (to values -1, based on two list that
    # define index and one that defines shape
    print(nv, ne, e_list, x_list)
    g_sparse = spmatrix(-1, x_list, e_list, (nv * nv, ne + 1))

    # Initialise optimization parameters
    h = matrix(-1.0, (nv, nv))
    c = matrix([0.0] * ne + [1.0])

    # Solve the convex optimization problem
    # Should raise here on windows
    sol = sdp(c, Gs=[g_sparse], hs=[h])
    assert sol is not None

[eddiebergman]

Okay, at this point I think it's a cvxopt/lapack issue as essentially the failures are stochastic, they seem to fail randomly on windows on different versions, sometimes succeeding for no reason I can discern. The github runners that are running the setup and tests seem to be the same so I'm not entirely sure why this is happening at all.

Test 1 (Success on 3.7, 3.8)

• https://github.com/eddiebergman/GraKeL/actions/runs/3183814445/jobs/5191482147

Test 2 (Failed all)

• https://github.com/eddiebergman/GraKeL/actions/runs/3183886579/jobs/5191640524

Assuming the generated sdp problem given is correct, my current thoughts on solutions:

• Try be more specific with the sdp solver and maybe try out different parameters which lead to something more stable.
• Raise an issue with cvxopt and wait however long that takes.
• Release anyway but not for windows

My preference is definitely for the last one given my reasons for updating GraKeL but I understand this is not ideal. I think all solutions are do-able in parallel, i.e

if windows: 
    solver = partial(sdp, something_windows_specific=...)
    # OR
    raise RuntimeError("LovazsKerenl not available for Windows, see: <issue link>")  
else:
   solver = sdp

sol = solver(C, Gs=[g_sparse], hs=[h])

I will raise an issue with cvxopt anyways with all the details and tag you in there as well in the meantime

Best, Eddie

[ysig]

Hi @eddiebergman

Sorry for jumping in a bit late. That's amazing work and thank you so much for pinning down this problem.

Lovasz has always been a pain due to numerical instability in windows.

To give you some further context, essentially what happens is the following:

• every kernel is psd. Thus the sdp solver shouldn't fail by the math. It is definetely a numerical error problem and I think it would be great if you raise an issue on cvxopt.

So I suggest the following:

• cvxopt is slow on development so ofc there is no time to waste.
• lovasz is not used that much as a kernel anyway as it's kinda slow so I suggest we comment out the unit tests of windows (for the faulty versions) on our current grakel wheels and through a warning when the kernel is loaded if the distro is windows and python is >= 3.9
• so I propose if possible to help me setup all these other jumbo mumbo (git actions and codecov) so at least all of it is functional and we can push the new version as soon as possible :))

[eddiebergman]

Hi @ysig,

So I think it's finally done: tests, wheels

Tests

I skip the lovasz tests on windows specifically and also inserted some code to raise a warning when using it on windows.

The only step left you to do would be to sign up on codecov.io which provides free test coverage reporting for open-source code. You would just have to enable it for this repo and that should be it (You do not need a token as it's a public repo).

To run tests locally, as pytest is now used, I've added it as a "dev" dependancy and they can be invoked by simply typing pytest anywhere in the repository. I defer you to pytest documentation for any more customization of it if needed.

Wheels

Everything seems to be building fine except some slight exceptions which are documented on the workflow. I would recommend you have a quick look through the workflow to see if things make sense for you there or if anything is not clear.

When the workflow is done and everything passes, all the compiled wheels will be available here and by clicking on the Artificat. This is a zipped up download of all the wheels. You can test them manually on your own machine first for piece of mind.

I would first merge everything into the main repository and then let them run and download from this repo, rather than my fork but the content should be identical.

Workflows in General

Since you may not be familiar with github action workflows, they are just .yaml files that are located in your repo under .github/workflows. They describe automated CI procedures for github to do and under what circumstances. I've enabled some sane options for test.yaml and release.yaml, basically having it so wheels are only generated on a push to the master branch or to a X.X.X version branch. Feel free to change this to what makes sense to you. For testing, they are trigged on any push to master or any push to a PR targeting master. Both of these can also be manually triggered using the Actions tab on github.

Future Releases

When Python 3.11/3.12 come out and are mainstream, and all the dependancies have updated (i.e. scikitlearn, scipy, etc...) then you can quite easily add a wheel for this be appending to this line. Likewise for testing here. I'm also happy to provide support for any of this in the future :)

Let me know if there's any more explanation you'd like or worries you may have!

Best, Eddie

[ysig]

Hi @eddiebergman thank you so much!

Two final things I would like you to add before I merge this PR is:

1. Previously we had this script that would upload the build wheels and sdist on pypi https://github.com/ysig/GraKeL/blob/0.1a8/ci_scripts/travis/success.sh can you add this on the wheel builders? I can for sure add the PYPI token

2. it would be great to have a commit keyword to skip uploading

   • equivalently we could have an action to push wheels that can only be triggered by the click of a contributor of the library - me in this case - but I'm not sure whether this is possible.

Lmk if sth doesn't make sense.

[eddiebergman]

Hi @ysig,

I added something from Pypa that does it now, I went with the easiest manual solution. Here's the action code for reference

On the actions tab, you will see the possibility to manually run the release.yaml workflow on any branch you have. There will be a checkbox (lower right of image) which will deploy to PyPI if everything runs correctly. The one downside is that this means you will likely run the workflow twice, once on a push to master automatically, and once when you manually select to release.

As you mentioned before, you will have to add a secret to you github acount which the documentation from the action specifies and you can find here.

If you're not happy with this solution and want it more automated then there are some other possibilities, for example, can trigger only a push to <branch> with a <tag> starting with a particular <substr>. I would advise against trying to automate it since the library is not under active development. I would guess that going to figure out what the automation trigger is not anymore convenient then just triggering it manually from github.

[eddiebergman]

Small update:

You can see in this workflow in the last step "Upload Release" that it seems to be working as intended. It fails for me as I have no access to PyPI/GraKeL and I also have no access token set. If you look at the log for it, you'll see the wheels and dist that it checks and tries to upload.

I think it's fully mergable then, other than I will resolve the install.sh conflict.

Solved, and also I removed the appveyor and Travis shields from the README