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

sage: fix dependencies on Darwin/Clang16 #298957

Merged
merged 11 commits into from
Mar 26, 2024
Merged

sage: fix dependencies on Darwin/Clang16 #298957

merged 11 commits into from
Mar 26, 2024

Conversation

Feyorsh
Copy link
Contributor

@Feyorsh Feyorsh commented Mar 25, 2024

Description of changes

Split off from #264126.

The majority of these changes are minor patches to make code C++17 compliant; c8d30c0 removes the FlintQS package which has been entirely superseded and should no longer be used.
Also, we might want to remove aarch64-darwin from the platforms of Sympow (14c15253922082e021a23514b40bb89c92026755) and fix the Sympow dependence in Sage, but that's an issue for a different PR.

Things done

  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandboxing enabled in nix.conf? (See Nix manual)
    • sandbox = relaxed
    • sandbox = true
  • Tested, as applicable:
  • Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
  • Tested basic functionality of all binary files (usually in ./result/bin/)
  • 24.05 Release Notes (or backporting 23.05 and 23.11 Release notes)
    • (Package updates) Added a release notes entry if the change is major or breaking
    • (Module updates) Added a release notes entry if the change is significant
    • (Module addition) Added a release notes entry if adding a new NixOS module
  • Fits CONTRIBUTING.md.

Add a 👍 reaction to pull requests you find important.

It appears m4rie only builds properly on Apple Clang; turning off
optimizations seems to resolve the failing test cases. See
https://bitbucket.org/malb/m4rie/issues/23/trying-to-compile-on-apple-m1
FlintQS is no longer maintained and has several bugs.
As of sagemath/sage#35419, all of FlintQS's
functionality is contained within Sage.
Usage of `std::bool_constant` prevents compilation using Clang; see
linbox-team/givaro#225
Internal logging macro shadows std's `log2`, so we rename it.
Adds newer patches upstreamed from Fedora into Sage; CRLF endings in the
`dietz` solver require `dos2unix` to apply the patches properly.
Frankly, I have no idea how this previously compiled on gcc.
@Feyorsh
Copy link
Contributor Author

Feyorsh commented Mar 25, 2024

@collares

@ofborg ofborg bot added the 6.topic: darwin Running or building packages on Darwin label Mar 25, 2024
@collares collares self-requested a review March 25, 2024 16:25
@ofborg ofborg bot added 8.has: clean-up 8.has: package (new) This PR adds a new package labels Mar 25, 2024
@ofborg ofborg bot requested review from timokau, omasanori and 7c6f434c March 25, 2024 16:40
@collares
Copy link
Member

collares commented Mar 25, 2024

Thanks for splitting the patches! Could you paste the output log of building sympow without the relaxed test?
In the other PR, the sympow configure output you pasted claims that the FPU supports double double precision (105 bits), but I find that highly unlikely. In a sense, Apple Silicon should be "easier" for sympow to support since it doesn't support extended precision (80-bit) floating point arithmetic, just the 64-bit doubles sympow requires.

Maybe this is an artifact of compiling the test program fpubits with -O3? The configure script claims it forces -O3 to ensure fused multiply-adds are being emitted, but maybe Clang is being even cleverer since the resulting output is "too precise". You can try seeing if a patch like https://gitweb.gentoo.org/repo/gentoo.git/tree/sci-mathematics/sympow/files/sympow-2.023.6-dont-force-O3.patch helps.

Copy link
Member

@collares collares left a comment

Choose a reason for hiding this comment

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

Besides the sympow investigation and the GAP upgrade which we probably should postpone, I think this is ready to be merged!

@collares
Copy link
Member

Testing results:

(Mildly related: We pass a bunch of flags when building fflas-ffpack on x86_64, and I think the original intention was to disable extra instruction sets but cf7b63d#diff-674d5fa234d898a81bc18975ded922bc971cfee426b7c25a8d1ca42b09c716a7 inadvertently changed it to enable instruction sets. Maybe we should drop the x86_64 special case entirely since there's the --without-archnative flag now.)

@collares
Copy link
Member

On x86_64-darwin, ofBorg reports the error below while building sympow. The error looks like it comes from PARI.

  ***   Warning: new stack size = 268435456 (256.000 Mbytes).
  ***   at top-level: ...,-(-1)^l/(l-1)!*polcoeff(F(k),-l,X)*L^(l-1))*X
  ***                                             ^---------------------
  ***   in function F: if(k%2==0,J(k-2,X)/1!*J(k/2-1,X/2)*sinv(k,X),s
  ***                            ^------------------------------------
  ***   not a function in function call
  ***   at top-level: K=40;Ds=vector(K);Ds[1]=DERIV(P);for(i=2,K,Ds[
  ***                                         ^----------------------
  ***   not a function in function call
  ***   at top-level: ...0;while(log(10^(-70)+abs(ev(P,MM,N)))>-148,MM*
  ***                                             ^---------------------
  ***   not a function in function call
  ***   at top-level: ...TAFILE,STR,",",precision(IEEE(MM),18))
  ***                                             ^-------------
  ***   in function IEEE: ...h(x)<2,return(0));y=ceil(log(abs(x))/log(2));i
  ***                                                 ^---------------------
  *** log: domain error in log: series valuation != 0
  ***   at top-level: ...0;while(log(10^(-70)+abs(ch(2^l,20)))<-148,l+=
  ***                                             ^---------------------
  ***   in function ch: ev(P,x,C)-ev(P,x,C+20)
  ***                   ^----------------------
  ***   not a function in function call
  ***   at top-level: while(2^l<MM,while(log(10^(-70)+abs(ch(2*2^l,n
  ***                          ^-------------------------------------
  *** _<_: forbidden comparison t_SER , t_POL.
  ***   at top-level: coeffs(0)
  ***                 ^---------
  ***   in function coeffs: ...or(i=0,19,print("PS ",i);QD(polcoeff(polcoeff(
  ***                                                   ^---------------------
  ***   in function QD: local(A);A=[IEEE(x),0,0,0];A[2]=IEEE(x-A[1]);A
  ***                               ^----------------------------------
  ***   in function IEEE: ...h(x)<2,return(0));y=ceil(log(abs(x))/log(2));i
  ***                                                 ^---------------------
  *** log: domain error in log: series valuation != 0
  ***   Warning: new stack size = 268435456 (256.000 Mbytes).
  ***   at top-level: ...,-(-1)^l/(l-1)!*polcoeff(F(k),-l,X)*L^(l-1))*X
  ***                                             ^---------------------
  ***   in function F: .../2)*sinv(k,X),sqrt(Pi)/2*J(k-1,X)/1!*J(k-1,X)/
  ***                                              ^---------------------
  ***   not a function in function call
  ***   at top-level: K=40;Ds=vector(K);Ds[1]=DERIV(P);for(i=2,K,Ds[
  ***                                         ^----------------------
  ***   not a function in function call
  ***   at top-level: ...0;while(log(10^(-70)+abs(ev(P,MM,N)))>-148,MM*
  ***                                             ^---------------------
  ***   not a function in function call
  ***   at top-level: ...TAFILE,STR,",",precision(IEEE(MM),18))
  ***                                             ^-------------
  ***   in function IEEE: ...h(x)<2,return(0));y=ceil(log(abs(x))/log(2));i
  ***                                                 ^---------------------
  *** log: domain error in log: series valuation != 0
  ***   at top-level: ...0;while(log(10^(-70)+abs(ch(2^l,20)))<-148,l+=
  ***                                             ^---------------------
  ***   in function ch: ev(P,x,C)-ev(P,x,C+20)
  ***                   ^----------------------
  ***   not a function in function call
  ***   at top-level: while(2^l<MM,while(log(10^(-70)+abs(ch(2*2^l,n
  ***                          ^-------------------------------------
  *** _<_: forbidden comparison t_SER , t_POL.
  ***   at top-level: coeffs(0)
  ***                 ^---------
  ***   in function coeffs: ...or(i=0,19,print("PS ",i);QD(polcoeff(polcoeff(
  ***                                                   ^---------------------
  ***   in function QD: local(A);A=[IEEE(x),0,0,0];A[2]=IEEE(x-A[1]);A
  ***                               ^----------------------------------
  ***   in function IEEE: ...h(x)<2,return(0));y=ceil(log(abs(x))/log(2));i
  ***                                                 ^---------------------
  *** log: domain error in log: series valuation != 0
**ERROR** P02L not found in param_data file in second round

No hardware support for 80-bit floating point (long double) on
aarch64-darwin; patch upstreamed
Can be safely reverted after updating to 1.7.1 or beyond
texinfo4 fails to compile on aarch64-darwin, and singular is no longer
constrained to use an older version
@collares
Copy link
Member

I've rewritten a few commit titles to match package names flint3 and python311Packages.fpylll, and dropped the sympow and gap patches so this PR can be merged sooner. Feel free to continue the sympow/gap discussion, either here or in the other PR. Thank you very much for your contribution!

@collares collares changed the title Sage: Fix dependencies on Darwin/Clang16 sage: fix dependencies on Darwin/Clang16 Mar 26, 2024
@collares collares merged commit d3fd636 into NixOS:master Mar 26, 2024
7 of 8 checks passed
@Feyorsh
Copy link
Contributor Author

Feyorsh commented Mar 26, 2024

Thanks for the help! I will take a look at sympow in more detail when I have the time.

@Feyorsh
Copy link
Contributor Author

Feyorsh commented Mar 31, 2024

Maybe this is an artifact of compiling the test program fpubits with -O3?

Explicitly setting -ffp-contract=off no longer results in a warning — I looked at how MacPorts packages sympow, and it appears they restrict to a very old version of Clang to avoid the default -ffp-contract=on.

CFLAGS for SYMPOW:  -std=gnu17 -fno-fast-math -ffp-contract=off 
The double precision of your FPU is 53 bits.

On x86_64-darwin, ofBorg reports the error below while building sympow.

The generated .gp files don't seem to be well formed, but I guess there are no errors on Linux (including aarch64-linux?). Nothing seems to be wrong with PARI (which, incidentally, is also compiled with -ffp-contract=off, and outputs The internal word representation of a double is not needed (64bit) during its configurePhase). Unfortunately generate.c, which generates the .gp files, is effectively obfuscated (at least for me), so fixing these errors could be difficult.

I'm out of ideas at the moment, but would be happy to circle back to this if anyone could suggest a new lead.

@collares
Copy link
Member

Malformed how? Can you upload one malformed .gp file?

@Feyorsh
Copy link
Contributor Author

Feyorsh commented Apr 1, 2024

After running nix-build sympow -K, the contents of imd.gp:

\p 250
N=600; mx=1; dv=0; L; X; SP=mx+2; default(seriesprecision,SP+1); H=vector(SP);
{for(i=1,SP,H[i]=vector(N);
 for(j=1,N,if(i==1,if(j==1,H[1][1]=1,H[i][j]=H[i][j-1]+1.0/j),
           if(j==1,H[i][1]=H[i-1][1],H[i][j]=H[i][j-1]+H[i-1][j]/j*1.0))));}
Hf(n,k)=if(k==0,0,H[n][k])
ZETA(n)=if(n==1,Euler,zeta(n))
J(k,v)=if(k<0,(v-k-1)*J(k+1,v),\
                1.0*(-1)^k/k!/v*\
                exp(sum(n=1,SP,(-1)^n*(ZETA(n)/n)*v^n))*\
                (1+sum(n=1,SP,Hf(n,k)*v^n)))
sinv(k,v)=if(k==0,1/v,-1/k-sum(l=1,SP,v^l/k^(l+1)))
two1ms(k,v)=2^(1+k)*sum(l=0,SP,log(1/2)^l/l!*v^l)

{DERIV(M)= sum(i=0,poldegree(M,L),
                (deriv(polcoeff(M,i,L),X)+(i+1)*polcoeff(M,i+1,L)/X)*L^i)}
        
{ev(T,v,C)=U=0; for(i=0,mx,U+=(truncate(polcoeff(T,i,L)+O(X^C))*L^i));
 subst(subst(U,L,log(v)),X,v);}

P=sum(k=0,N,sum(l=1,mx+1,-(-1)^l/(l-1)!*polcoeff(F(k),-l,X)*L^(l-1))*X^k);
K=40; Ds=vector(K); Ds[1]=DERIV(P); for(i=2,K,Ds[i]=DERIV(Ds[i-1]));

ieee(x)=(round(x<<53)*1.0)>>53
{IEEE(x)=if(x==0,return(0));if(length(x)<2,return(0));
         y=ceil(log(abs(x))/log(2));ieee(x/2^y)*2^y;}
{QDc(x)=local(A); A=[IEEE(x),0]; A[2]=IEEE(x-A[1]); A=precision(A,18);
 print(A[1]); print(A[2]); print(0); print(0);}

{doit(x,C)=print("AT ",x); QDc(ev(P,x,C));
 for(d=1,35,print("DERIV ",d); QDc(ev(Ds[d]/d!,x,C)));}
ch(x,C)=ev(P,x,C)-ev(P,x,C+20)

setup(a,b)=for(i=0,31,doit(2^a+2^a*i/32,b));
print("About to find TOO_BIG"); \\ ends up in /dev/null
MM=100; while(log(10^(-34)+abs(ev(P,MM,N)))>-74,MM*=1.1);
write1("datafiles/param_data",STR,",",precision(IEEE(MM),18));
print("Now working backwards..."); \\ ends up in /dev/null
l=-10; while(log(10^(-34)+abs(ch(2^l,20)))<-74,l+=1); l-=1; s=l;
write1("datafiles/param_data",",",l-5); n=20;
print("Starting to write mesh files"); \\ ends up in /dev/null

dSTR=concat("datafiles/",STR); STRm=concat(dSTR,"M.txt");
default(logfile,STRm); default(log,1);
while(2^l*31/32<MM,\
      while(log(10^(-34)+abs(ch(2*2^l,n)))>-74,n+=5); setup(l,n);l+=1);
write("datafiles/param_data",",",l-s);
coeffs(j)=for(i=0,19,print("PS ",i); QDc(polcoeff(polcoeff(P,j,L),i,X)));
coO(j)=for(i=0,9,print("PSL ",2*i+1);QDc(polcoeff(polcoeff(P,j,L),2*i+1,X)));
coE(j)=for(i=0,9,print("PSL ",2*i);QDc(polcoeff(polcoeff(P,j,L),2*i,X)));

STRs=concat(dSTR,"S.txt"); default(logfile,STRs); default(log,1);
coeffs(0); if(doEV,coE(1),coO(1));
default(log,0);

I could be mistaken on imd.gp being malformed; I'm unsure if the not a function in function call warning can be dismissed or is indicative of a larger issue. It appears the only "critical" error is

  ***   in function IEEE: ...h(x)<2,return(0));y=ceil(log(abs(x))/log(2));i
  ***                                                 ^---------------------
  *** log: domain error in log: series valuation != 0

which might be caused PARI using more bits of precision than Sympow. They're compiled with more or less the same flags, but I believe the test programs used to check the FPU are different.

@collares
Copy link
Member

collares commented Apr 2, 2024

Thanks for the information. I think the entry point is the armd.gp file, which defines the F function; if you run imd.gp directly, you'll get the not a function in function call error (I also get it on x86_64-linux).

The weird thing from the log I posted above is that it complains about not finding the J, DERIV and ev functions, which is clearly defined in imd.gp. If I run the following script:

STR="M02H"; doEV=1;
F(k)=if(k%2==0,J(k-2,X)/1!*J(k/2-1,X/2)*sinv(k,X),\
    sqrt(Pi)/2*J(k-1,X)/0!*J(k-2,X)/1!*1/J((k-1)/2,X/2)*two1ms(k,X)*sinv(k,X))
\p 250
N=600; mx=1; dv=0; L; X; SP=mx+2; default(seriesprecision,SP+1); H=vector(SP);
{for(i=1,SP,H[i]=vector(N);
 for(j=1,N,if(i==1,if(j==1,H[1][1]=1,H[i][j]=H[i][j-1]+1.0/j),
           if(j==1,H[i][1]=H[i-1][1],H[i][j]=H[i][j-1]+H[i-1][j]/j*1.0))));}
Hf(n,k)=if(k==0,0,H[n][k])
ZETA(n)=if(n==1,Euler,zeta(n))
J(k,v)=if(k<0,(v-k-1)*J(k+1,v),\
                1.0*(-1)^k/k!/v*\
                exp(sum(n=1,SP,(-1)^n*(ZETA(n)/n)*v^n))*\
                (1+sum(n=1,SP,Hf(n,k)*v^n)))
sinv(k,v)=if(k==0,1/v,-1/k-sum(l=1,SP,v^l/k^(l+1)))
two1ms(k,v)=2^(1+k)*sum(l=0,SP,log(1/2)^l/l!*v^l)

{DERIV(M)= sum(i=0,poldegree(M,L),
                (deriv(polcoeff(M,i,L),X)+(i+1)*polcoeff(M,i+1,L)/X)*L^i)}
        
{ev(T,v,C)=U=0; for(i=0,mx,U+=(truncate(polcoeff(T,i,L)+O(X^C))*L^i));
 subst(subst(U,L,log(v)),X,v);}

P=sum(k=0,N,sum(l=1,mx+1,-(-1)^l/(l-1)!*polcoeff(F(k),-l,X)*L^(l-1))*X^k);
K=40; Ds=vector(K); Ds[1]=DERIV(P); for(i=2,K,Ds[i]=DERIV(Ds[i-1]));

ieee(x)=(round(x<<53)*1.0)>>53
{IEEE(x)=if(x==0,return(0));print(x);print(length(x));if(length(x)<2,return(0));
         y=ceil(log(abs(x))/log(2));ieee(x/2^y)*2^y;}
{QDc(x)=local(A); A=[IEEE(x),0]; A[2]=IEEE(x-A[1]); A=precision(A,18);
 print(A[1]); print(A[2]); print(0); print(0);}

{doit(x,C)=print("AT ",x); QDc(ev(P,x,C));
 for(d=1,35,print("DERIV ",d); QDc(ev(Ds[d]/d!,x,C)));}
ch(x,C)=ev(P,x,C)-ev(P,x,C+20)

setup(a,b)=for(i=0,31,doit(2^a+2^a*i/32,b));
print("About to find TOO_BIG"); \\ ends up in /dev/null
MM=100; while(log(10^(-34)+abs(ev(P,MM,N)))>-74,MM*=1.1);

print(precision(IEEE(MM),18));
print("Now working backwards..."); \\ ends up in /dev/null
l=-10; while(log(10^(-34)+abs(ch(2^l,20)))<-74,l+=1); l-=1; s=l;
print(l-5);

which is basically the definition of F plus a prefix of imd.gp with a few extra print statements in IEEE, I get the following output on x86-64:

$ gp -q -s 300000000 test.gp 
   realprecision = 250 significant digits
About to find TOO_BIG
285.3116706110000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000001
13
285.3116706110000109
Now working backwards...
-7

Do you already see weird output at this point, or does imd.gp only misbehave later on?

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.

2 participants