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

upgrade lrcalc to 2.1 #31355

Closed
sagetrac-tmonteil mannequin opened this issue Feb 7, 2021 · 136 comments
Closed

upgrade lrcalc to 2.1 #31355

sagetrac-tmonteil mannequin opened this issue Feb 7, 2021 · 136 comments

Comments

@sagetrac-tmonteil
Copy link
Mannequin

sagetrac-tmonteil mannequin commented Feb 7, 2021

lrcalc 2.1 is out https://bitbucket.org/asbuch/lrcalc/src/master/ChangeLog

Instead of rewriting Sage's Python bindings to support the new API, we replace them with a wrapper around the new upstream provided bindings

Upstream:

lrcalclib-2.1.tar.gz (lrcalc): https://sites.math.rutgers.edu/~asbuch/lrcalc/lrcalc-2.1.tar.gz
lrcalc-2.1.tar.gz (lrcalc_python): https://pypi.io/packages/source/l/lrcalc/lrcalc-2.1.tar.gz

CC: @antonio-rojas @anneschilling @fchapoton @tscrim @thierry-FreeBSD @orlitzky @mkoeppe @kiwifb @asbuch @slel

Component: packages: optional

Keywords: upgrade, lrcalc

Author: Antonio Rojas, Matthias Koeppe

Branch/Commit: de4fe09

Reviewer: Matthias Koeppe, Travis Scrimshaw

Issue created by migration from https://trac.sagemath.org/ticket/31355

@sagetrac-tmonteil sagetrac-tmonteil mannequin added this to the sage-9.3 milestone Feb 7, 2021
@sagetrac-tmonteil

This comment has been minimized.

@sagetrac-tmonteil sagetrac-tmonteil mannequin changed the title upgrade lrslib to 2.0 upgrade lrcalc to 2.0 Feb 7, 2021
@sagetrac-tmonteil
Copy link
Mannequin Author

sagetrac-tmonteil mannequin commented Feb 7, 2021

Branch: u/tmonteil/upgrade_lrcalc_to_2_0

@sagetrac-tmonteil
Copy link
Mannequin Author

sagetrac-tmonteil mannequin commented Feb 7, 2021

Commit: 850f9bc

@sagetrac-tmonteil
Copy link
Mannequin Author

sagetrac-tmonteil mannequin commented Feb 7, 2021

comment:4

I get the following error when doctesting:

ImportError: /opt/sagemath/sage/local/lib/python3.7/site-packages/sage/libs/lrcalc/lrcalc.cpython-37m-x86_64-linux-gnu.so: undefined symbol: st_new

New commits:

850f9bc#31355 : upgrade lrcalc to 2.0

@antonio-rojas
Copy link
Contributor

comment:5

This has major API changes. Large parts of lrcalc.pyx needs to be rewritten because the used functions have been removed or modified.

@sagetrac-tmonteil
Copy link
Mannequin Author

sagetrac-tmonteil mannequin commented Feb 7, 2021

comment:6

I see, any help welcome !

@orlitzky
Copy link
Contributor

comment:8

The existing spkg-configure.m4 needs to be updated to detect v2.0 (once it actually works), too.

@nthiery
Copy link
Contributor

nthiery commented Feb 16, 2021

comment:9

Some information from Anders:

I recently put out a new version of my lrcalc program, much of it is
rewritten, and some of the most important functions are close to twice as fast
as before.  Some special cases have also been optimized to run a lot faster.

I have included some (pure) Python3 bindings in the repository, so I think
this update will be easy.  But I don't understand Sage well enough to do it
myself.

For reference, some pointers:

@asbuch
Copy link

asbuch commented Feb 16, 2021

comment:10

The Python bindings are only in the repository and a bit raw, so probably only useful as examples. But I expect they are easier to start from than the .h files.

@antonio-rojas
Copy link
Contributor

@antonio-rojas
Copy link
Contributor

comment:12

Replying to @asbuch:

The Python bindings are only in the repository and a bit raw, so probably only useful as examples. But I expect they are easier to start from than the .h files.

Actually, they are almost equivalent to the current bindings in Sage. The attached branch replaces lrcalc.pyx with a thin wrapper over lrcalc's bindings.

It mostly just translates the output from lrcalc to sage notation. The only function of lrcalc.pyx that isn't straghtforward to port is quantum multiplication: I found no way to recover the q-grading from lrcalc's output, so this is reimplemented in the patch.

The branch needs an update of lrcalc to include python bindings.


New commits:

6596bbdReplace lrcalc.pyx with a wrapper over lrcalc's own Python bindings

@antonio-rojas
Copy link
Contributor

Changed commit from 850f9bc to 6596bbd

@antonio-rojas
Copy link
Contributor

comment:13

Replying to @asbuch:

The Python bindings are only in the repository and a bit raw, so probably only useful as examples. But I expect they are easier to start from than the .h files.

Since you are here and I can not find a bug tracker for lrcalc: the setup.py is missing metadata, so the egg for lrcalc gets installed as UNKNOWN-0.0.0.0

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Feb 21, 2021

Branch pushed to git repo; I updated commit sha1. New commits:

0a9490aMerge branch 'develop' of git://git.sagemath.org/sage into t/31355/upgrade_lrcalc_to_2_0
26f7f26Fix oversight in q_degrees formula

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Feb 21, 2021

Changed commit from 6596bbd to 26f7f26

@asbuch
Copy link

asbuch commented Feb 21, 2021

comment:15

If my bindings are used, then that will make it easier to keep things compatible if I make more changes to the C interface. I should be able to keep the Python interface stable, at least up to reordering of output.

Quantum function: I will add a function mult_quantum_pairs that returns a dict from ((partition),d) to integers, you can switch to that if you want.

setup.py: Does the following include the tags you are looking for? Suggestions welcome!

setup(name='lrcalc',
    version='2.0.0',
    description='Littlewood-Richardson Calculator',
    long_description='Littlewood-Richardson Calculator',
    url='https://math.rutgers.edu/~asbuch/lrcalc',
    author='Anders Skovsted Buch',
    license='GPL3',
    ext_modules = cythonize([
        Extension("lrcalc", ["lrcalc.pyx"],
                  libraries=["lrcalc"])
    ])
)

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Feb 21, 2021

Changed commit from 26f7f26 to 627192a

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Feb 21, 2021

Branch pushed to git repo; I updated commit sha1. New commits:

627192aUse Sage integers in results

@antonio-rojas
Copy link
Contributor

comment:17

Replying to @asbuch:

setup.py: Does the following include the tags you are looking for? Suggestions welcome!

setup(name='lrcalc',
    version='2.0.0',
    description='Littlewood-Richardson Calculator',
    long_description='Littlewood-Richardson Calculator',
    url='https://math.rutgers.edu/~asbuch/lrcalc',
    author='Anders Skovsted Buch',
    license='GPL3',
    ext_modules = cythonize([
        Extension("lrcalc", ["lrcalc.pyx"],
                  libraries=["lrcalc"])
    ])
)

That works, thanks. Also, although maybe it is a little bit too late for that, it would be good to bump the soversion of the C shared library since the API changed.

@asbuch
Copy link

asbuch commented Feb 21, 2021

comment:18

Replying to @antonio-rojas:

Replying to @asbuch:

setup.py: Does the following include the tags you are looking for? Suggestions welcome!

setup(name='lrcalc',
    version='2.0.0',
    description='Littlewood-Richardson Calculator',
    long_description='Littlewood-Richardson Calculator',
    url='https://math.rutgers.edu/~asbuch/lrcalc',
    author='Anders Skovsted Buch',
    license='GPL3',
    ext_modules = cythonize([
        Extension("lrcalc", ["lrcalc.pyx"],
                  libraries=["lrcalc"])
    ])
)

That works, thanks. Also, although maybe it is a little bit too late for that, it would be good to bump the soversion of the C shared library since the API changed.

I changed the version to "2.0.0" and thought that was in line with this page. Please let me know if I am doing something wrong.

@antonio-rojas
Copy link
Contributor

comment:19

Replying to @asbuch:

I changed the version to "2.0.0" and thought that was in line with this page. Please let me know if I am doing something wrong.

You need to change the -version-info in https://bitbucket.org/asbuch/lrcalc/src/master/src/Makefile.am#lines-23

@asbuch
Copy link

asbuch commented Feb 21, 2021

comment:20

Replying to @antonio-rojas:

Replying to @asbuch:

I changed the version to "2.0.0" and thought that was in line with this page. Please let me know if I am doing something wrong.

You need to change the -version-info in https://bitbucket.org/asbuch/lrcalc/src/master/src/Makefile.am#lines-23

I see, thanks! According to conventions, is that line in Makefile.am independent from this line in configure.am:

AC_INIT([lrcalc],[2.0.0],[asbuch at math rutgers edu])

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Feb 22, 2021

Branch pushed to git repo; I updated commit sha1. New commits:

52c120fUse new mult_quantum degrees option in lrcalc 2.1 for quantum multiplication

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Feb 22, 2021

Changed commit from 627192a to 52c120f

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Feb 22, 2021

Branch pushed to git repo; I updated commit sha1. New commits:

e13f155Upgrade lrcalc to 2.1 and install python bindings

@antonio-rojas
Copy link
Contributor

comment:95

Ping. Anything else needed here?

@dimpase
Copy link
Member

dimpase commented Dec 12, 2021

comment:96

What is the minimum lrcalc version required by lrcalc_python?

@antonio-rojas
Copy link
Contributor

comment:97

lrcalc and lrcalc_python come from the same repo, so I suppose the versions need to match.

@dimpase
Copy link
Member

dimpase commented Dec 12, 2021

comment:98

A number of distrbutions have system-wide lrcalc installations, some still version 1.*, and those can be picked up by Sage.

If these are not compatible they should not be used.

@antonio-rojas
Copy link
Contributor

comment:99

1.* will not be picked up, the ivector.h header was introduced in 2.0. Don't know about 2.0, according to repology only gentoo is shipping that. @kiwifb, any reason it hasn't been updated to 2.1?

@kiwifb
Copy link
Member

kiwifb commented Dec 12, 2021

comment:100

I just have been quite busy. And since @orlitzky has migrated a lot of packages to the main tree, I feel less personal pressure to update. We have 2.0 in the main tree but not 2.1 yet. I will have to see what needs to be done.

@kiwifb
Copy link
Member

kiwifb commented Dec 12, 2021

comment:101

lrcalc_python/SPKG.rst claims it is GPL2+ but https://bitbucket.org/asbuch/lrcalc/src/master/python/ and https://bitbucket.org/asbuch/lrcalc/src/master/python/setup.py make it pretty clear that it is GPL3.

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Dec 12, 2021

Branch pushed to git repo; I updated commit sha1. New commits:

03e0c8fFix spkg-configure to use C and only pass for version 2.1
b944affFix license

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Dec 12, 2021

Changed commit from 42ecd9a to b944aff

@antonio-rojas
Copy link
Contributor

comment:103

Replying to @dimpase:

A number of distrbutions have system-wide lrcalc installations, some still version 1.*, and those can be picked up by Sage.

If these are not compatible they should not be used.

Actually spkg-configure didn't work at all because it was using C++ instead of C. Fixed it now and added a check for a header only present in 2.1 so it will fail with older versions.

Fixed the license too, should be ready to go now.

@kiwifb
Copy link
Member

kiwifb commented Dec 12, 2021

comment:104

I have opened a PR for getting 2.1 in Gentoo and I have an ebuild ready for the python bindings for the sage-on-gentoo overlay just waiting for the PR inclusion before going live [because of dependencies requirements, cannot go in until lrcalc-2.1 is present].

@mkoeppe mkoeppe modified the milestones: sage-9.5, sage-9.6 Dec 21, 2021
@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Dec 24, 2021

Changed commit from b944aff to 5d443b9

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Dec 24, 2021

Branch pushed to git repo; I updated commit sha1. New commits:

5d443b9Merge remote-tracking branch 'origin/develop' into t/31355/upgrade_lrcalc_to_2_0

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jan 9, 2022

Branch pushed to git repo; I updated commit sha1. New commits:

ffb08d8Merge remote-tracking branch 'origin' into t/31355/upgrade_lrcalc_to_2_0

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jan 9, 2022

Changed commit from 5d443b9 to ffb08d8

@antonio-rojas
Copy link
Contributor

comment:108

Ping, hope to get this in 9.6

@tscrim
Copy link
Collaborator

tscrim commented Feb 23, 2022

@tscrim

This comment has been minimized.

@tscrim
Copy link
Collaborator

tscrim commented Feb 23, 2022

Changed commit from ffb08d8 to de4fe09

@tscrim
Copy link
Collaborator

tscrim commented Feb 23, 2022

comment:109

Does Sage's auto-download mechanism automatically rename the tarballs to their appropriate values. I am worried that both tarballs (lib and python) are called lrcalc-2.1.tar.gz on their respective upstream locations.

I made a few other little tweaks and checked the tests. If my changes are good, then it is a positive review.


New commits:

e8d0b5bMerge branch 'u/arojas/upgrade_lrcalc_to_2_0' of git://trac.sagemath.org/sage into u/arojas/upgrade_lrcalc_to_2_0
de4fe09Some improvements to lrcalc.py.

@tscrim
Copy link
Collaborator

tscrim commented Feb 23, 2022

Changed reviewer from Matthias Koeppe, ... to Matthias Koeppe, Travis Scrimshaw

@mkoeppe
Copy link
Contributor

mkoeppe commented Feb 23, 2022

comment:110

Replying to @tscrim:

Does Sage's auto-download mechanism automatically rename the tarballs to their appropriate values. I am worried that both tarballs (lib and python) are called lrcalc-2.1.tar.gz on their respective upstream locations.

Yes, it is renamed to the value given in the tarball field of checksums.ini. See comment:80.

@antonio-rojas
Copy link
Contributor

comment:111

Thanks

@vbraun
Copy link
Member

vbraun commented Feb 27, 2022

Changed branch from u/tscrim/upgrade_lrcalc_2_1-31355 to de4fe09

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

10 participants