#28410 closed defect (fixed)

Bug with docker images sagemath/sagemath-dev:develop and sagemath/sagemath:develop

Reported by: mercatp Owned by:
Priority: major Milestone: sage-9.0
Component: linear algebra Keywords:
Cc: vdelecroix, saraedum, cpernet, fbissey Merged in:
Authors: Julian Rüth Reviewers: Vincent Delecroix
Report Upstream: N/A Work issues: test docker image
Branch: 6c90f38 (Commits, GitHub, GitLab) Commit: 6c90f38e6bec6635d929331ebed9c1e7374aefcd
Dependencies: #28041 Stopgaps:

Status badges

Description (last modified by mercatp)

When we run the docker image with

$docker run -it sagemath/sagemath-dev:develop

the following code fail:

sage@56153d827f26:~/sage$ sage
┌────────────────────────────────────────────────────────────────────┐
│ SageMath version 8.9.beta8, Release Date: 2019-08-25               │
│ Using Python 2.7.15. Type "help()" for help.                       │
└────────────────────────────────────────────────────────────────────┘
┏━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━┓
┃ Warning: this is a prerelease version, and it may be unstable.     ┃
┗━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━┛
Setting permissions of DOT_SAGE directory so only you can read and write it.
sage: m = identity_matrix(2)
sage: m.charpoly()
---------------------------------------------------------------------------
SignalError                               Traceback (most recent call last)
<ipython-input-2-caa4c4748463> in <module>()
----> 1 m.charpoly()

/home/sage/sage/local/lib/python2.7/site-packages/sage/matrix/matrix_integer_dense.pyx in sage.matrix.matrix_integer_dense.Matrix_integer_dense.charpoly (build/cythonized/sage/matrix/matrix_integer_dense.c:12819)()
   1337         elif algorithm == 'linbox':
   1338             g = (<Polynomial_integer_dense_flint> PolynomialRing(ZZ, names=var).gen())._new()
-> 1339             sig_on()
   1340             linbox_fmpz_mat_charpoly(g.__poly, self._matrix)
   1341             sig_off()

SignalError: Illegal instruction
sage: 

The bug is also present with the docker image sagemath/sagemath:develop

$ docker run -it sagemath/sagemath:develop
┌────────────────────────────────────────────────────────────────────┐
│ SageMath version 8.9.beta8, Release Date: 2019-08-25               │
│ Using Python 2.7.15. Type "help()" for help.                       │
└────────────────────────────────────────────────────────────────────┘
┏━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━┓
┃ Warning: this is a prerelease version, and it may be unstable.     ┃
┗━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━┛
Setting permissions of DOT_SAGE directory so only you can read and write it.
sage: identity_matrix(2).charpoly()
---------------------------------------------------------------------------
SignalError                               Traceback (most recent call last)
<ipython-input-1-e3f0f98f29ff> in <module>()
----> 1 identity_matrix(Integer(2)).charpoly()

/home/sage/sage/local/lib/python2.7/site-packages/sage/matrix/matrix_integer_dense.pyx in sage.matrix.matrix_integer_dense.Matrix_integer_dense.charpoly (build/cythonized/sage/matrix/matrix_integer_dense.c:12819)()
   1337         elif algorithm == 'linbox':
   1338             g = (<Polynomial_integer_dense_flint> PolynomialRing(ZZ, names=var).gen())._new()
-> 1339             sig_on()
   1340             linbox_fmpz_mat_charpoly(g.__poly, self._matrix)
   1341             sig_off()

SignalError: Illegal instruction
sage: 

Change History (16)

comment:1 Changed 21 months ago by mercatp

  • Component changed from PLEASE CHANGE to linear algebra
  • Type changed from PLEASE CHANGE to defect

comment:2 Changed 21 months ago by mercatp

  • Cc vdelecroix added

comment:3 Changed 21 months ago by vdelecroix

  • Cc saraedum cpernet added

Most likely, the docker image is built with a set of CPU instructions that is not supported by all computers. That can be because the built script is not careful about that or that some library (here libox) are just ignoring some configuration variables...

comment:4 follow-up: Changed 21 months ago by cpernet

Indeed, givaro, fflas_ffpack and linbox detect available instruction sets at configure time. You can force disable them by passing a --disable-XXX argument to the call to configure in the spkg-install files. I assume the following sequence would do the job for portability on any x86 or i686 up to nehalem:

 --disable-avx2 --disable-avx --disable-fma --disable-fma4

You may add --disable-sse42 --disable-sse41 --disable-ssse3 --disable-sse3 if you want to support older micro-archs.

comment:5 Changed 21 months ago by mercatp

  • Description modified (diff)
  • Summary changed from Bug with docker image sagemath/sagemath-dev:develop to Bug with docker images sagemath/sagemath-dev:develop and sagemath/sagemath:develop

comment:6 in reply to: ↑ 4 Changed 20 months ago by saraedum

Sorry, I had not seen this ticket (though I am on CC.)

Replying to cpernet:

Indeed, givaro, fflas_ffpack and linbox detect available instruction sets at configure time. You can force disable them by passing a --disable-XXX argument to the call to configure in the spkg-install files. I assume the following sequence would do the job for portability on any x86 or i686 up to nehalem:

 --disable-avx2 --disable-avx --disable-fma --disable-fma4

You may add --disable-sse42 --disable-sse41 --disable-ssse3 --disable-sse3 if you want to support older micro-archs.

I believe we should use the flags that we use on conda https://github.com/conda-forge/linbox-feedstock/blob/master/recipe/build.sh. I remember we did some research there so these are very widely supported.

Do you know how we can pass these flags without patching the SPKG?

comment:7 Changed 20 months ago by saraedum

There's LINBOX_CONFIGURE, FFLAS_FFPACK_CONFIGURE, GIVARO_CONFIGURE. Let's set them in the docker build.

comment:8 Changed 20 months ago by saraedum

  • Dependencies set to #28041

I need some docker build fixes from #28041 to be able to test this in CI.

comment:9 Changed 20 months ago by saraedum

Actually, we build the docker images with SAGE_FAT_BINARY=yes:

https://gitlab.com/sagemath/sage/blob/master/docker/Dockerfile#L159

comment:10 Changed 20 months ago by saraedum

The build log for 8.9.beta8 that you were using says:

[linbox-1.6.3] -----------------------------------------------
[linbox-1.6.3]         START  LINBOX CONFIG                   
[linbox-1.6.3] -----------------------------------------------
[linbox-1.6.3] Detecting SIMD instruction set
[linbox-1.6.3] SSE disabled
[linbox-1.6.3] SSE2 disabled
[linbox-1.6.3] SSE3 disabled
[linbox-1.6.3] SSSE3 disabled
[linbox-1.6.3] SSE4.1 disabled
[linbox-1.6.3] SSE4.2 disabled
[linbox-1.6.3] AVX disabled
[linbox-1.6.3] AVX2 disabled
[linbox-1.6.3] FMA3 disabled
[linbox-1.6.3] FMA4 disabled

However, fflas-ffpack says:

[fflas_ffpack-2.4.3] Detecting SIMD instruction set
[fflas_ffpack-2.4.3] SSE disabled
[fflas_ffpack-2.4.3] SSE2 disabled
[fflas_ffpack-2.4.3] SSE3 disabled
[fflas_ffpack-2.4.3] SSSE3 disabled
[fflas_ffpack-2.4.3] SSE4.1 disabled
[fflas_ffpack-2.4.3] SSE4.2 disabled
[fflas_ffpack-2.4.3] AVX disabled
[fflas_ffpack-2.4.3] AVX2 disabled
[fflas_ffpack-2.4.3] AVX512F enabled
[fflas_ffpack-2.4.3] AVX512VL enabled
[fflas_ffpack-2.4.3] AVX512DQ enabled
[fflas_ffpack-2.4.3] FMA3 disabled
[fflas_ffpack-2.4.3] FMA4 disabled

Could that be the problem?

comment:11 Changed 20 months ago by saraedum

  • Branch set to u/saraedum/28410

comment:12 Changed 20 months ago by saraedum

  • Authors set to Julian Rüth
  • Commit set to 6c90f38e6bec6635d929331ebed9c1e7374aefcd
  • Status changed from new to needs_review
  • Work issues set to test docker image

Once #28041 has been merged, I can schedule a manual pipeline from GitLab's web interface. Once that has completed, mercatp should check that this actually makes the docker image work for him :)


New commits:

6c90f38Build FAT_BINARY without avx512* instruction sets

comment:13 follow-up: Changed 20 months ago by gh-mwageringel

  • Cc fbissey added

There was a related discussion on sage-release a few weeks ago.

comment:14 in reply to: ↑ 13 Changed 20 months ago by fbissey

Replying to gh-mwageringel:

There was a related discussion on sage-release a few weeks ago.

Yes indeed, and that last commit is what is needed from that discussion. If I remember correctly the macros are all included in givaro, fflas-ffpack and linbox. Currently most of them only have an effect on fflas-ffpack but it would be a good idea to extend fat binary setting across all three packages.

comment:15 Changed 20 months ago by vdelecroix

  • Milestone changed from sage-8.9 to sage-9.0
  • Reviewers set to Vincent Delecroix
  • Status changed from needs_review to positive_review

comment:16 Changed 20 months ago by vbraun

  • Branch changed from u/saraedum/28410 to 6c90f38e6bec6635d929331ebed9c1e7374aefcd
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.