Opened 3 years ago
Closed 3 years ago
#21341 closed enhancement (fixed)
Better wrapping of IML
Reported by:  Bouillaguet  Owned by:  

Priority:  major  Milestone:  sage7.4 
Component:  interfaces  Keywords:  sd75 
Cc:  cpernet  Merged in:  
Authors:  Charles Bouillaguet  Reviewers:  Jeroen Demeyer 
Report Upstream:  Reported upstream. No feedback yet.  Work issues:  
Branch:  08277ea (Commits)  Commit:  08277eae873e15b3c9c3bbe6818c7b95d6121090 
Dependencies:  Stopgaps: 
Description (last modified by )
The IML package should be better wrapped up, for instance in sage/libs/iml
. The functions are hidden in sage/matrix/matrix_integer_dense.pyx
.
There is a problem that IML conflicts with linbox: https://github.com/linboxteam/linbox/issues/35
For instance, adding :
cdef extern from "linbox/solutions/solve.h" namespace "LinBox": pass
on top of sage/matrix/matrix_integer_dense.pyx
prevents it from compiling.
Change History (18)
comment:1 Changed 3 years ago by
 Description modified (diff)
 Keywords sd75 added
comment:2 Changed 3 years ago by
 Description modified (diff)
comment:3 Changed 3 years ago by
 Branch set to u/Bouillaguet/iml_wrapper
comment:4 Changed 3 years ago by
 Commit set to 574b4389929f9a444362e88894aff95432cb913c
 Status changed from new to needs_review
comment:5 Changed 3 years ago by
 Status changed from needs_review to needs_work
What is the point of this:
cdef long iml_nullspaceMP(long n, long m, const mpz_t *A, mpz_t * *mp_N_pass): return nullspaceMP(n, m, A, mp_N_pass) cdef iml_nonsingSolvLlhsMM(SOLU_POS solupos, long n, long m, mpz_t *mp_A, mpz_t *mp_B, mpz_t mp_N, mpz_t mp_D): nonsingSolvLlhsMM(solupos, n, m, mp_A, mp_B, mp_N, mp_D)
Why not just call the IML functions directly?
If it's related to the problem in the ticket description, I'd rather try to fix that problem or at least understand why it does not work.
comment:6 Changed 3 years ago by
 Description modified (diff)
comment:7 Changed 3 years ago by
 Description modified (diff)
comment:8 Changed 3 years ago by
I think this is an upstream linbox bug.
comment:9 Changed 3 years ago by
 Description modified (diff)
 Report Upstream changed from N/A to Reported upstream. No feedback yet.
comment:10 Changed 3 years ago by
In code like
cdef enum SOLU_POS: LeftSolu = 101 RightSolu = 102
it's better to not put the values (101 and 102) since this confuses people to think that Cython actually uses those values (it just ignores them). Also: use 4 spaces of indentation.
comment:11 Changed 3 years ago by
 Commit changed from 574b4389929f9a444362e88894aff95432cb913c to 08277eae873e15b3c9c3bbe6818c7b95d6121090
Branch pushed to git repo; I updated commit sha1. New commits:
08277ea  move access to IML code into sage.libs.iml

comment:12 Changed 3 years ago by
 Status changed from needs_work to needs_review
Jeroen, does this suit you (also see my "private" email)?
comment:13 Changed 3 years ago by
Let's see what the patchbot says (you can remind me if the patchbot tests pass).
comment:14 Changed 3 years ago by
And fill in your name as author...
comment:16 Changed 3 years ago by
Jeroen, the tests fail on the patchbot for an unrelated reason (a deprecation warning in generic_graph.pyx). I can't reproduce the problem on my machine. How do you feel about the ticket?
comment:17 Changed 3 years ago by
 Reviewers set to Jeroen Demeyer
 Status changed from needs_review to positive_review
The patchbot failures are indeed unrelated.
comment:18 Changed 3 years ago by
 Branch changed from u/Bouillaguet/iml_wrapper to 08277eae873e15b3c9c3bbe6818c7b95d6121090
 Resolution set to fixed
 Status changed from positive_review to closed
New commits:
move access to IML code into sage.libs.iml