-
-
Notifications
You must be signed in to change notification settings - Fork 482
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
Allow using arb, flint and ntl from Homebrew #30745
Comments
This comment has been minimized.
This comment has been minimized.
comment:2
upstream ecl is indeed useless, as no known to me distro (gentoo, homebrew) provides sufficiently patched ecl-20. perhaps ntl/flint/singular combo on homebrew is good enough for Sage? |
comment:3
Traditionally build systems are supposed to pick up stuff in /usr/local, so its a bit of a feature. Gentoo doesn't install anything there afaik, it is intended for stuff that users install by hand (i.e. non-portage in Gentoo). I consider it the big homebrew WTF. Realistically, if you have stuff installed in /usr/local then you don't have a supported platform any more. Sure we should try to accommodate homebrew but imho not a blocker. |
comment:4
Replying to @vbraun:
Given that since Sage 9.1, we recommend homebrew packages that the user should install, I don't think the view that a system with homebrew installed in Setting it to "critical" rather than "blocker" because there is, of course, an easy remedy - just uninstall the useless homebrew packages that cause the problem. |
This comment has been minimized.
This comment has been minimized.
comment:5
Replying to @vbraun:
Despite your personal misgivings about Homebrew, this is a non-starter, Volker, sorry. Homebrew is the most popular macOS package manager out there, and it goes without saying that we want to support as many packages there as possible, and hopefully have a proper Homebrew Sage package. As well, many *BSD systems install their packages in /usr/local, e.g. FreeBSD (where we now have a semi-working native package, with all the deps neatly divided into native packages), OpenBSD, etc. |
comment:6
Trying this with
which indeed has |
comment:7
A related recent change was made in #29697 ( |
comment:8
The correct fix for |
comment:9
arb, ntl, and flint can and should just be enabled on Homebrew. ECL is a different story, as it lacks patches. |
comment:10
It's not quite that easy, I am afraid. There are still the threading issues such as #27764 |
comment:11
Replying to @mkoeppe:
these issues have solved themselves - I am able to build and doctest on macOS 10.15.7 with arb, ntl, flint from Homebrew just fine |
comment:12
more precisely, I see 2 doctests failing in |
Branch: u/dimpase/build/brewenablearbetc |
comment:14
my tests show that we can use Homebrew's NTL, arb, and flint, and with flint 2.6.3 patches in it's time to advertise them. For testing, don't forget to ran New commits:
|
Commit: |
Author: Dima Pasechnik |
comment:18
The branch from #29719 is named |
comment:19
I've tried on another computer (running OS X 10.15.7 this time), including the branch from #29719:
Most of these are due to Python 3.9, but this is new:
(Details in the previous comment.) Not only is it new, but I see this error (a) with this branch (merged with #29719), but also (b) with homebrew's flint, ntl, and arb installed but with a standard |
comment:20
yes, I also see the error on Anyhow, arb has nothing to do with Singular, and I fail to see differences between ntl+flint from Homebrew and ntl+flint 2.6.3 from #29719. That ntl and flint install in Homebrew affect (lib)Singular linked with other instances of ntl and flint is a Singular bug. I'll report it. |
Dependencies: #29719 |
comment:22
the --- a/src/sage/rings/polynomial/plural.pyx
+++ b/src/sage/rings/polynomial/plural.pyx
@@ -367,6 +367,8 @@ cdef class NCPolynomialRing_plural(Ring):
True
sage: H is loads(dumps(H)) # indirect doctest
True
+ sage: A2.<x,y,z> = FreeAlgebra(GF(5), 3)
+ sage: R2 = A2.g_algebra({y*x:x*y-z, z*x:x*z+2*x, z*y:y*z-2*y}, order=TermOrder('degrevlex', 2))
Check that :trac:`17224` is fixed::
If you like I can add this "fix" to the branch. alternatively, if I just change the order of tests, using ./sage -tp --random-seed=0 --randorder=42 --long src/sage/rings/polynomial/plural.pyx |
comment:23
That's sweeping things under the rug. Not necessarily the worst thing to do, but I wish there were an actual fix. I guess if we "fix" this now and upstream fixes the real bug pretty soon, it's a good approach. |
comment:24
Replying to @jhpalmieri:
Yes, certainly, what I propose is not a real fix. Just reproducing the Heisenbug in question outside of the testing framework is a very tall order, leave alone fixing it. By the way, if I run the tests with |
Branch pushed to git repo; I updated commit sha1. New commits:
|
comment:26
ready for review |
comment:27
This whole thing makes no sense to me. I agree that the problem is there with #29719. If instead of your change, I do this: diff --git a/src/sage/rings/polynomial/plural.pyx b/src/sage/rings/polynomial/plural.pyx
index c2792aec88..500253533a 100644
--- a/src/sage/rings/polynomial/plural.pyx
+++ b/src/sage/rings/polynomial/plural.pyx
@@ -399,6 +399,8 @@ cdef class NCPolynomialRing_plural(Ring):
sage: R1 = A1.g_algebra({y*x:x*y-z, z*x:x*z+2*x, z*y:y*z-2*y}, order=TermOrder('degrevlex', 2))
sage: A2.<x,y,z> = FreeAlgebra(GF(5), 3)
sage: R2 = A2.g_algebra({y*x:x*y-z, z*x:x*z+2*x, z*y:y*z-2*y}, order=TermOrder('degrevlex', 2))
+ sage: A2.<x,y,z> = FreeAlgebra(GF(5), 3)
+ sage: R2 = A2.g_algebra({y*x:x*y-z, z*x:x*z+2*x, z*y:y*z-2*y}, order=TermOrder('degrevlex', 2))
sage: A3.<x,y,z> = FreeAlgebra(GF(11), 3)
sage: R3 = A3.g_algebra({y*x:x*y-z, z*x:x*z+2*x, z*y:y*z-2*y}, order=TermOrder('degrevlex', 2))
sage: A4.<x,y,z> = FreeAlgebra(GF(13), 3) then I get a more detailed segfault:
Does that convey any useful information? |
This comment has been minimized.
This comment has been minimized.
comment:28
The problem is certainly not in flint update, which is modern high quality code, but in Singular's plural package, which is relying on 9-year old C++, not used much, either, no wonder it misbehaves. I cc few people who might tell us more about the dump (I can reproduce it, so it's not random). It seems that Singular with Clang is not tested much (e.g. Singular built on OpenBSD 6.7 and 6.8 with OpenBSD's clang 10 and 11: standalone Singular crashes on startup). |
comment:29
At first glance, from what I can read, it looks like the system has to be "primed" before the error occurs. That suggests memory management issues - which have always been a hard topic in singular :( |
comment:30
Unfortunately it also seems that the Singular team is uninterested in portability testing - my pull request at Singular/Singular#1018 has been sitting there without even an acknowledgment by the maintainers |
comment:31
If this is really a Singular issue, then let's merge the bandaid and move on. Any objections? What is the status with ecl? Does that need a new ticket? |
comment:32
Also note #29528 which is about this Singular bug. As well, in #25993 (upgrade to Singular 4.1.3p2 - the latest) |
comment:33
Replying to @jhpalmieri:
yes, let's move on this. And open a new ticket for ecl. |
Changed reviewer from https://github.com/mkoeppe/sage/actions/runs/367237067 to John Palmieri, https://github.com/mkoeppe/sage/actions/runs/367237067 |
Changed reviewer from John Palmieri, https://github.com/mkoeppe/sage/actions/runs/367237067 to John Palmieri |
comment:36
Since the ECL part is postponed, should we Could the summary be: "Allow using arb, flint and ntl from Homebrew"? Is the ECL issue part of #29617 or does it need a new ticket? |
comment:37
Changing ticket summary; feel free to improve or revert. |
comment:38
Replying to @slel:
yes, #29617 would probably take care of this. |
Changed branch from u/dimpase/build/brewenablearbetc to |
/usr/local
is leaking into our build, through wrong orders of include and/or library directives.Critical for 9.3 because it affects a major supported platform. We should at least give a configure-time error if these installations are found on homebrew if we cannot fix this.
Depends on #29719
CC: @dimpase @jhpalmieri @vbraun @nbruin @kiwifb
Component: build: configure
Author: Dima Pasechnik
Branch/Commit:
2fd95cd
Reviewer: John Palmieri
Issue created by migration from https://trac.sagemath.org/ticket/30745
The text was updated successfully, but these errors were encountered: