sage: don't import stuff in low level functions

[malb]

See https://groups.google.com/g/sage-devel/c/iIttQjz-Zaw/m/Gs9BnTLsBQAJ

[dimpase]

Why does fpylll even need to know that it's called from Sage?

[malb]

Because we need to import and export Sage Integers.

[dimpase]

can't you use gmpy2 ?

[malb]

Not as far as I can tell: we do Sage integers only for i/o with Sage. We should use something like gmpy2 for arbitrary precision floating point numbers but I guess that's out of scope for this PR.

[dimpase]

I/O with Sage? Do you mean that for some strange reason you accept Sage intergers as input, and can return results of this type, too?

I don't think it's a good design, to be frank.
Why can't we instead move the corresponding pieces into Sage, so that Sage calls fpylll with its native data, whatever it is.

[malb]

Yep. This is because I have no interest in writing wrappers for every FPyLLL function that takes Integers in Sage. I write 10 lines in FPyLLL or 10 times more of boilerplate in Sage.

[dimpase]

10 times? As far as I can see, Sage only ever imports BKZ, LLL, SVP, IntegerMatrix, and load_strategies_json from fpylll.
And of these, AFAICS, only IntegerMatrix has to do anything with taking Sage's ZZ.

[dimpase]

fpylll is not alone in trying to figure out whether it's called from Sage, also mpmath does something like this... @fredrik-johansson

[malb]

FPyLLL isn't only in Sage to drive Sage's high-level functions but also to make experimenting with lattice reduction in Sage easy. So this should work:

sage: from fpylll import IntegerMatrix
sage: A = IntegerMatrix(10, 10)
sage: A[0,0] = 17
sage: A[0,0]
17

FWIW: FPyLLL is underutilised even in Sage's highlevel functions because I lack the resources at the moment to hook it in nicely everywhere.

[CRTified]

I've applied this patch on my copy of nixpkgs, and this solves the problem from the sage-devel discussion (I'll write an answer there, too):

2.1 New sage version:
+ /nix/store/wwfz03cv8p0v7rvcbw1mkxn93d7blr7q-sage-9.6/bin/sage /nix/store/mjzl7bi3d6v03gyiv29cpwv7l4wk3sl3-fpylll-regression.py
  sage: SageMath version 9.6, Release Date: 2022-05-15
    py: 3.10.5 (main, Jun  6 2022, 12:05:50) [GCC 11.3.0]
fpylll: 0.5.7
LLL took 0.583477 seconds.

2.2 New sage version, py mode:
+ /nix/store/wwfz03cv8p0v7rvcbw1mkxn93d7blr7q-sage-9.6/bin/sage --python /nix/store/mjzl7bi3d6v03gyiv29cpwv7l4wk3sl3-fpylll-regression.py
  sage: SageMath version 9.6, Release Date: 2022-05-15
    py: 3.10.5 (main, Jun  6 2022, 12:05:50) [GCC 11.3.0]
fpylll: 0.5.7
LLL took 0.660228 seconds.

2.3 New py3 version:
+ /nix/store/ixj3bpnhvidyfb6ndll3dyjmfvgvy9q5-python3-3.10.5-env/bin/python3 /nix/store/mjzl7bi3d6v03gyiv29cpwv7l4wk3sl3-fpylll-regression.py
    py: 3.10.5 (main, Jun  6 2022, 12:05:50) [GCC 11.3.0]
fpylll: 0.5.7
LLL took 0.674341 seconds.

2.4 Show fplll dependecy for new sage:
+ nix why-depends /nix/store/wwfz03cv8p0v7rvcbw1mkxn93d7blr7q-sage-9.6 /nix/store/nz882q6rv16pwg8b76h7q36va113gjij-fplll-5.4.2 | cat
/nix/store/wwfz03cv8p0v7rvcbw1mkxn93d7blr7q-sage-9.6
└───/nix/store/lqhjkbbdmmhpip4zg0iixpqnznr1wrd6-sage-with-env-9.6
    └───/nix/store/b5c8kf90ifhzw3csi6zyrcmqyxg83a2q-python3.10-sagelib-9.6
        └───/nix/store/bzlscxam2xbg4c56v6q2pldn1dyp4qr1-python3.10-fpylll-0.5.7
            └───/nix/store/nz882q6rv16pwg8b76h7q36va113gjij-fplll-5.4.2

2.5 Show fplll dependecy for new py3:
+ nix why-depends /nix/store/ixj3bpnhvidyfb6ndll3dyjmfvgvy9q5-python3-3.10.5-env /nix/store/nz882q6rv16pwg8b76h7q36va113gjij-fplll-5.4.2 | cat
/nix/store/ixj3bpnhvidyfb6ndll3dyjmfvgvy9q5-python3-3.10.5-env
└───/nix/store/hcd0y7s6d9bql05rhg1vx9cgb7bjihzz-python3.10-fpylll-0.5.7
    └───/nix/store/nz882q6rv16pwg8b76h7q36va113gjij-fplll-5.4.2

Note that I've applied the patch only to the fpylll version used in sage, which is visible by the comparing the paths of the dependencies (/nix/store/bzlscxam2xbg4c56v6q2pldn1dyp4qr1-python3.10-fpylll-0.5.7 and /nix/store/hcd0y7s6d9bql05rhg1vx9cgb7bjihzz-python3.10-fpylll-0.5.7).

[dimpase]

sage: A[0,0] = 17

one can do e.g.

sage: A[0,0] = int(17)

and avoid the conversion dance all together.

[malb]

Yup, we can put that burden on the users :)