Closed malb closed 1 year ago
Why does fpylll
even need to know that it's called from Sage?
Because we need to import and export Sage Integers.
can't you use gmpy2
?
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.
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.
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.
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
.
fpylll
is not alone in trying to figure out whether it's called from Sage, also mpmath does something like this... @fredrik-johansson
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.
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
).
sage: A[0,0] = 17
one can do e.g.
sage: A[0,0] = int(17)
and avoid the conversion dance all together.
Yup, we can put that burden on the users :)
See https://groups.google.com/g/sage-devel/c/iIttQjz-Zaw/m/Gs9BnTLsBQAJ