Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

igraph: Update to 0.10.12, fix spkg-check; python_igraph: Update to 0.11.5, remove spkg-install workarounds #37819

Merged
merged 24 commits into from
Jun 9, 2024

Conversation

mkoeppe
Copy link
Contributor

@mkoeppe mkoeppe commented Apr 17, 2024

📝 Checklist

Increasing the lower bound for system cmake to 3.18. This will cause system cmake on ubuntu-focal to be rejected, so cmake will be built from source on this platform. https://repology.org/project/cmake/versions

  • The title is concise and informative.
  • The description explains in detail what this PR is about.
  • I have linked a relevant issue or discussion.
  • I have created tests covering the changes.
  • I have updated the documentation and checked the documentation preview.

⌛ Dependencies

Copy link

github-actions bot commented Apr 17, 2024

Documentation preview for this PR (built with commit 4f748fe; changes) is ready! 🎉
This preview will update shortly after each push to this PR.

@mkoeppe mkoeppe requested a review from dcoudert April 17, 2024 04:13
Copy link
Contributor

@dcoudert dcoudert left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have igraph 0.10.11 installed via homebrew

MAC-04017247:sage dcoudert$ ls -l /opt/homebrew/Cellar/igraph/
total 0
drwxr-xr-x  12 dcoudert  admin  384 17 avr 09:15 0.10.11

in a fresh shell session, I did:

make distclean
./bootstrap
source .homebrew-build-env
./configure
make build
./sage -i python_igraph

For some reason, it installs igraph from source and don't use my homebrew version.
How to fix that ?

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Apr 17, 2024

Thanks for testing! I had forgotten to check/update the spkg-configure file. Fixed now

@mkoeppe mkoeppe changed the title build/pkgs/igraph: Update to 0.10.11; fix spkg-check, remove python-igraph spkg-install workarounds igraph: Update to 0.10.11, fix spkg-check; python_igraph: Update to 0.11.4, remove spkg-install workarounds Apr 18, 2024
Copy link
Contributor

@dcoudert dcoudert left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Installation and tests ok on macOS 12.7.4.

LGTM.

@dcoudert
Copy link
Contributor

I have some doubts about all the failing CI. That's why I'm not seeing this PR to positive review yet.

@szhorvat
Copy link
Contributor

szhorvat commented May 7, 2024

igraph 0.10.12 and python-igraph 0.11.5 are released now. I suggest going directly for these new versions.

@mkoeppe mkoeppe added this to the sage-10.4 milestone May 14, 2024
Copy link
Contributor

@dcoudert dcoudert left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Works fine on macOS with homebrew igraph.

LGTM.

@dcoudert
Copy link
Contributor

dcoudert commented Jun 5, 2024

Some errors are reported by the CI on the uninstall part.

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Jun 5, 2024

Not sure what you mean by the "uninstall part" but there is a problem with cmake.
test / linux (fedora-30, standard)

#29 40.36   [igraph-0.10.12]   No spkg-legacy-uninstall script; nothing to do
#29 40.36   [igraph-0.10.12]   [spkg-install] Configuring igraph-0.10.12 with cmake
#29 40.36   [igraph-0.10.12]   [spkg-install] CMake Error at CMakeLists.txt:8 (cmake_minimum_required):
#29 40.36   [igraph-0.10.12]   [spkg-install]   CMake 3.18 or higher is required.  You are running version 3.17.2
#29 40.36   [igraph-0.10.12]   [spkg-install] 
#29 40.36   [igraph-0.10.12]   [spkg-install] 
#29 40.36   [igraph-0.10.12]   [spkg-install] -- Configuring incomplete, errors occurred!
#29 40.36   [igraph-0.10.12]   [spkg-install] ********************************************************************************
#29 40.36   [igraph-0.10.12]   [spkg-install] Error configuring igraph-0.10.12 with cmake
#29 40.36   [igraph-0.10.12]   [spkg-install] ********************************************************************************

@dcoudert
Copy link
Contributor

dcoudert commented Jun 5, 2024

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Jun 5, 2024

What I see there is this test failure:

sage -t --random-seed=302171850113617614625936987131470123946 src/sage/graphs/generators/random.py
#30 2631.5 **********************************************************************
#30 2631.5 File "src/sage/graphs/generators/random.py", line 1605, in sage.graphs.generators.random.RandomPartialKTree
#30 2631.5 Failed example:
#30 2631.5     G.treewidth()
#30 2631.5 Expected:
#30 2631.5     5
#30 2631.5 Got:
#30 2631.5     6
#30 2633.3

Do you know something about this test?

@dcoudert
Copy link
Contributor

dcoudert commented Jun 6, 2024

it seems to be

What I see there is this test failure:

sage -t --random-seed=302171850113617614625936987131470123946 src/sage/graphs/generators/random.py
#30 2631.5 **********************************************************************
#30 2631.5 File "src/sage/graphs/generators/random.py", line 1605, in sage.graphs.generators.random.RandomPartialKTree
#30 2631.5 Failed example:
#30 2631.5     G.treewidth()
#30 2631.5 Expected:
#30 2631.5     5
#30 2631.5 Got:
#30 2631.5     6
#30 2633.3

Do you know something about this test?

This seems to be an error from tdlib.

sage: from sage.graphs.graph_decompositions.tree_decomposition import width_of_tree_decomposition
sage: G = Graph('q~~|~q{mLhUo}?v?EO`{?h`?wo@w?fo?As_?lG?Cp_?Uc??{?_?|???Y_G?@gH??{?_?A{???Dp???Dk???Ax????l????Dh????Qo???GkG???_lG????Q_???I@p?G???uO????As????@@gH????FG??A??AW_??A??d????CG@w?C????PE?@????D@G?C???t?O??????')
sage: G.treewidth(algorithm='sage')
5
sage: G.treewidth(algorithm='tdlib')
6
sage: T = G.treewidth(algorithm='tdlib', certificate=True)
sage: width_of_tree_decomposition(G, T)
6

May be we should first try to update tdlib to see if it fixes this issue, and otherwise report upstream.

@dcoudert
Copy link
Contributor

dcoudert commented Jun 6, 2024

I have opened #38159

@dcoudert
Copy link
Contributor

dcoudert commented Jun 6, 2024

This PR looks good to me. Let me know if ready for review (currently in needs work).

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Jun 6, 2024

Thanks. It's ready for review

Copy link
Contributor

@dcoudert dcoudert left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Jun 6, 2024

Thank you!

@vbraun vbraun merged commit a09048c into sagemath:develop Jun 9, 2024
34 of 43 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants