Opened 2 years ago

Closed 2 years ago

#21341 closed enhancement (fixed)

Better wrapping of IML

Reported by: Bouillaguet Owned by:
Priority: major Milestone: sage-7.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 jdemeyer)

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/linbox-team/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 2 years ago by Bouillaguet

  • Description modified (diff)
  • Keywords sd75 added

comment:2 Changed 2 years ago by Bouillaguet

  • Description modified (diff)

comment:3 Changed 2 years ago by Bouillaguet

  • Branch set to u/Bouillaguet/iml_wrapper

comment:4 Changed 2 years ago by Bouillaguet

  • Commit set to 574b4389929f9a444362e88894aff95432cb913c
  • Status changed from new to needs_review

New commits:

574b438move access to IML code into sage.libs.iml

comment:5 Changed 2 years ago by jdemeyer

  • 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 2 years ago by jdemeyer

  • Description modified (diff)

comment:7 Changed 2 years ago by jdemeyer

  • Description modified (diff)

comment:8 Changed 2 years ago by jdemeyer

I think this is an upstream linbox bug.

comment:9 Changed 2 years ago by jdemeyer

  • Description modified (diff)
  • Report Upstream changed from N/A to Reported upstream. No feedback yet.

comment:10 Changed 2 years ago by jdemeyer

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 2 years ago by git

  • Commit changed from 574b4389929f9a444362e88894aff95432cb913c to 08277eae873e15b3c9c3bbe6818c7b95d6121090

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

08277eamove access to IML code into sage.libs.iml

comment:12 Changed 2 years ago by Bouillaguet

  • Status changed from needs_work to needs_review

Jeroen, does this suit you (also see my "private" email)?

comment:13 Changed 2 years ago by jdemeyer

Let's see what the patchbot says (you can remind me if the patchbot tests pass).

comment:14 Changed 2 years ago by jdemeyer

And fill in your name as author...

comment:15 Changed 2 years ago by Bouillaguet

  • Authors set to Charles Bouillaguet

Oops.

comment:16 Changed 2 years ago by Bouillaguet

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 2 years ago by jdemeyer

  • Reviewers set to Jeroen Demeyer
  • Status changed from needs_review to positive_review

The patchbot failures are indeed unrelated.

comment:18 Changed 2 years ago by vbraun

  • Branch changed from u/Bouillaguet/iml_wrapper to 08277eae873e15b3c9c3bbe6818c7b95d6121090
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.