HebiRobotics/MFL

Potential API improvements

Closed this issue · 5 comments

ennerf commented

Issue for notes about points where the API is confusing or cumbersome to use. Could be integrated into the next major release.

Char Strings

  • Mat5::newString is confusing as it creates a char-vector (='') rather than a MATLAB string object (""). It should probably be newCharString(String) or newChar(String)
  • Char:getString() returns a String if it is a char-vector. It's a bit too close to toString(). It may be better to name it getAsString() or asString() (similar to asCharSequence()).

Scalars

  • There are files with many scalars, and getMatrix("x").getDouble(0) is a bit cumbersome. It may be good to add a double = getScalar("name") overload or some other way to make that a bit cleaner. Maybe a Scalar interface that can be implemented by AbstractMatrix?
 double value = Mat5.readFile("data.mat").getScalar("x").asDouble()

Sparse matrices

  • Provide a way to create sparse matrices without going through EJML. Maybe a triplet structure.
  • Remove getNzMax() from API in next major release

MatFile

  • clear() should return this, similar to add and other methods

External Libs

  • maybe add some sort of Array-Converter/Wrapper interface for library interop, e.g.,

    • DMatrixMaj ejmlMatrix = matFile.get("fieldName", Mat5Ejml.DMatrixRMaj);
    • MatFile.registerConverter(Mat5Ejml.Converters).add("fieldName", ejmlMatrix);

  • maybe use a (global) plugin mechanism? Mat5.registerPlugins(Mat5Ejml.Plugin)

  • maybe add extensions for ojAlgo and JavaCV

ennerf commented

It may make sense to get rid of long indices.

The API was built with forward compatibility for -v7.3 (>2GB matrices) in mind, but after experimenting a bit with the HDF based 7.3 format we found that the state of HDF on Java is such a huge mess that we abandoned any plans on supporting it.

Also, in other projects we've recently observed significant performance gains (>100%) in some cases when switching from long to int (probably caused by more aggressive JIT optimizations), so it may be worth doing some performance benchmarks.

ennerf commented
  • It may make sense to have the subsystem not show up in the entry list. Very few users would need to know about it, and it can result in additional code when trying to ignore it. It could live in a separate variable that's accessible only via Mat5::getSubsystem(). See #48
  • 👍1
ennerf commented

@nedtwigg I just realized you upvoted hiding the Mat5Subsystem. I just addressed this in this commit. Let me know if you have any comments.

Also, now that it has been out for a pretty long time, have you noticed anything that'd be good to add or change?

  • 👍1
nedtwigg commented

Commit looks good. I think MFL is fantastic, only thing missing imo is 1.0 ;-)

  • 🎉1
ennerf commented

Yes to the 1.0, hence the question whether anything is missing ;-)

The main reason I've held off on it so far is because I'd like to change the indexing from long to int. It hasn't been an issue so far though, so it has been really low on my priority list... I promise I'll get there eventually!

  • 👍1