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

[RFC] New package: sagelib-9.5 #35339

Closed
wants to merge 2 commits into from
Closed

Conversation

tornaria
Copy link
Contributor

@tornaria tornaria commented Feb 1, 2022

This is a different approach for a sagemath package than #34030.

Here we build the sagelib as a python module and install it in the system site-packages. The scripts are all installed in /usr/lib/sagemath/bin to avoid polluting the PATH. A symlink /usr/bin/sagelib is provided.

NOTE: for the moment this is set up so it can coexists together with the sagemath package; that's why the package name is sagelib (instead of sagemath) and the binary is sagelib (instead of sage). The plan is to rename this package to sagemath and the binary to sage to replace the other since this one is much simpler and should work better.

With this version jupyter notebook should work out of the box without any adjustments. Please let me know if there's anything missing.

Any comments about packaging, package layout, etc, now is the moment. I would like to get this merged soon since everything seems to be in place.

Cc: @dkwo @leahneukirchen

@dkwo
Copy link
Contributor

dkwo commented Feb 1, 2022

Let me first say you've done a great job.
I'm building it right now, will report soon.
My only comment is about depends: is there a way not to bring in -devel and gcc- stuff?

@dkwo
Copy link
Contributor

dkwo commented Feb 1, 2022

For some reason, I struggle to pick up this kernel in jupyterlab.
I wonder if we need something similar to what Arch has

# fix symlinks to assets
  _pythonpath=`python -c "from sysconfig import get_path; print(get_path('platlib'))"`
  for _i in $(ls "$srcdir"/sage-$pkgver/src/sage/ext_data/notebook-ipython); do
    rm "$pkgdir"/usr/share/jupyter/kernels/sagemath/$_i
    ln -s $_pythonpath/sage/ext_data/notebook-ipython/$_i "$pkgdir"/usr/share/jupyter/kernels/sagemath/
  done

which also matches what we were doing previosuly, as well as suggested by sage docs
Otherwise, it builds and tests fine.

@tornaria
Copy link
Contributor Author

tornaria commented Feb 1, 2022

Let me first say you've done a great job. I'm building it right now, will report soon. My only comment is about depends: is there a way not to bring in -devel and gcc- stuff?

I only added stuff that is needed to pass doctests. There is some functionality to cython-compile code (e.g. you may want to define a function in cython inside a notebook) and that needs at least headers and sometimes libraries, and a working gcc (which is required indirectly via gcc-fortran).

It might be possible to add "features" to sage that correspond to presence of gcc and headers, etc. but I'm not sure this is worth the trouble. Although some sage developers are trying to move it in a "modularization" direction such that it's possible to split it in parts and in that sense it could make sense to split out a "sagemath-cython" subpkg that gives this ability, etc.

@tornaria
Copy link
Contributor Author

tornaria commented Feb 1, 2022

For some reason, I struggle to pick up this kernel in jupyterlab. I wonder if we need something similar to what Arch has

# fix symlinks to assets
  _pythonpath=`python -c "from sysconfig import get_path; print(get_path('platlib'))"`
  for _i in $(ls "$srcdir"/sage-$pkgver/src/sage/ext_data/notebook-ipython); do
    rm "$pkgdir"/usr/share/jupyter/kernels/sagemath/$_i
    ln -s $_pythonpath/sage/ext_data/notebook-ipython/$_i "$pkgdir"/usr/share/jupyter/kernels/sagemath/
  done

The jupyter kernel spec is fixed right at the top of the post_install().

which also matches what we were doing previosuly, as well as suggested by sage docs Otherwise, it builds and tests fine.

The goal is for the notebook to work out of the box. How are you running it?

I'm just doing

$ sagelib -notebook

and the browser is launched with a jupyter session in which I can create a new "SageMath 9.5" notebook. It is identical if run as $ jupyter notebook. It also works fine from jupyterlab (although jupyterlab does NOT work out of the box, see #35341)

Are you sure you don't have leftover cruft from "fixing" of jupyter for the other package? You shouldn't have anything at all in /usr/share/jupyter other than what is shipped in this sagelib package.

@dkwo
Copy link
Contributor

dkwo commented Feb 1, 2022

I removed /usr/local/share/jupyter as well as the other sagemath.
Now it works with no modification:

[I 2022-02-01 14:26:46.535 LabApp] JupyterLab extension loaded from /usr/lib/python3.10/site-packages/jupyterlab
[I 2022-02-01 14:26:46.535 LabApp] JupyterLab application directory is /usr/share/jupyter/lab

although I may have some leftovers here

ls -la /usr/share/jupyter
total 24
drwxr-xr-x  6 root root 4096 Dec  2 17:18 .
drwxr-xr-x 80 root root 4096 Jan 20 14:08 ..
drwxr-xr-x  4 root root 4096 Feb  1 10:52 kernels
drwxr-xr-x  8 root root 4096 May 14  2021 lab
drwxr-xr-x  3 root root 4096 Dec  2 17:18 nbconvert
drwxr-xr-x  3 root root 4096 Dec  2 17:18 nbextensions

only thing is it complais about many
[W 2022-02-01 14:26:55.623 LabApp] Build recommended

@tornaria
Copy link
Contributor Author

tornaria commented Feb 1, 2022

As for tests:

  1. there is one test failure in all architectures when running doctest on an installed package, namely:
sage -t --long --warn-long 60.0 --random-seed=36734515608409952297316154028352847160 /usr/lib/python3.10/site-packages/sage/misc/sageinspect.py
**********************************************************************
File "/usr/lib/python3.10/site-packages/sage/misc/sageinspect.py", line 186, in sage.misc.sageinspect.loadable_module_extension
Failed example:
    sage.structure.sage_object.__file__.endswith(loadable_module_extension())
Expected:
    True
Got:
    False
**********************************************************************

This is just because loadable_module_extension() returns ".cpython-310-x86_64-linux-gnu.so" but we rename these files to just .so (in this case .../sage/structure/sage_object.cpython-310-x86_64-linux-gnu.so becomes .../sage/structure/sage_object.so). A simple workaround is to patch loadable_module_extension() to just return ".so" but that's not upstream-friendly.

  1. There is a fairly deterministic numerical noise in maxima on i686:
sage -t --long --warn-long 60.0 --random-seed=43442452431567652147900536515427073750 /usr/lib/python3.10/site-packages/sage/interfaces/maxima_abstract.py
**********************************************************************
File "/usr/lib/python3.10/site-packages/sage/interfaces/maxima_abstract.py", line 1527, in sage.interfaces.maxima_abstract.MaximaAbstractElement.?
Failed example:
    maxima('exp(-sqrt(x))').nintegral('x',0,1)
Expected:
    (0.5284822353142306, 4.1633141378838...e-11, 231, 0)
Got:
    (0.5284822353142306, 4.163291933423352e-11, 231, 0)
**********************************************************************
1 item had failures:
   1 of   5 in sage.interfaces.maxima_abstract.MaximaAbstractElement.?
    [236 tests, 1 failure, 4.57 s]

This seems to be just the error bound for the integral (the value seems to be exactly the same, more than 4e-11 close in any case), so not a real issue safe and should be easy to fix and upstream. I am guessing the reason we didn't see this before is that in the other package we are running maxima -l ecl but here we are running maxima which brings up sbcl version of maxima (note: we still need maxima-ecl because it's the only one that can be used as a library; here we are doctesting the pexpect interface to maxima which can run either).

I expect two small patches will make everything pass. If we figure out the issues @dkwo is having with the notebook (which works fine for me), I think we'd have something ready to merge.

@leahneukirchen: can you test in your webapp?

@dkwo
Copy link
Contributor

dkwo commented Feb 1, 2022

If I remove sagelib and jupyterlab, and by hand

rm -r /usr/share/jupyter/lab
rm -r /usr/share/jupyter/kernels/ (empty)

(probably again leftovers), then reinstall them, I'm on the same page as you, so this is an issue with jupyterlab, not sagelib.
Ready to merge imo.

@leahneukirchen
Copy link
Member

Works as a jupyter notebook without any adjustments.

I updated docker.io/leahneukirchen/voidsage:9.5.

@tornaria
Copy link
Contributor Author

tornaria commented Feb 1, 2022

I pushed the fix for the two doctest failures.

If everything works ok I plan on redoing this PR later today as a package named sagemath with /usr/bin/sage as the main binary.

@leahneukirchen
Copy link
Member

And then we can merge it right? :)

@tornaria
Copy link
Contributor Author

tornaria commented Feb 2, 2022

I moved back to the sagemath branch and PR (#34030) which should now be ready to merge. Remember to remove and cleanup this sagelib package before installing the new sagemath package since they will have file conflicts.

@tornaria tornaria closed this Feb 2, 2022
@tornaria tornaria deleted the sagelib branch February 3, 2022 00:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants