Skip to content

[ODK] Code clean up summary

A. Breust edited this page Nov 7, 2018 · 1 revision

NOTE: This page is meant to be updated according to the identification notes from the project board.

template<class Vector, class Blackbox, class SolveMethod, class DomainCategory>
Vector& solve(Vector& x,
              const Blackbox& A,
              const Vector& b,
              const DomainCategory& tag,
              const SolveMethod& M);
  • Blackbox might not be a BB per se;
  • DomainCategory is reached through FieldTraits<typename Blackbox::Field>::categoryTag() which points to struct ClassifyRing<Field>::categoryTag which goes down to things like RingCategories::RationalTag (or IntegerTag, ModularTag);
  • SolveMethod is something like Method::Hybrid, Method::Elimination, Method::CRA.

Bugs

  • Determine what this is (by bboyer in 61f986733ef5f66d02a81c449892b1daf91bca24):
//x @todo temporary - fix this
//#define inBlasRange(p) true

Design issues

  • solve.h There are some definitions of solvein, but they still take (Vector& x, Matrix& A, const Vector& b). They modify the Matrix in place, but they are not defined from everything. We should discuss about making the in-place method by default? Whatever, I would expect the solvein to be in-place for the vector, not the matrix.
  • Choosing between Method::Elimination and Method::CRA feels strange. In CRA, what method does it uses underlying? Fact is, it is done via Method::CRA which is a CRATraits which holds an iterationMethod.
    • @ClementPernet says that this concept works fine, we could make Method::MPI -> Method::CRA or Method::DixonLifting -> iterationMathod.
  • Why is there a specific solveCRA?
  • We currently have no way to say go through MPI, it just uses it if __LINBOX_HAVE_MPI is defined. We should think about how to keep this behavior by default, but allow user switch.

Code duplication

  • Dimension checking could be done once? Should not be done in both algorithms and solutions at least.
  • rational-solver.h and rational-solver2.h (-_-')
  • rational-reconstruction.h and rational-reconstruction2.h

Test-coverage lack

  • Should write a test that try all combinaisons of SolveMethod/Matrix.

Benchmark coverage lack

Gold nuggets

Clone this wiki locally