AKEngels / CAST

Conformational Analysis and Search Tool
GNU Lesser General Public License v3.0
1 stars 5 forks source link

RFC: Retire the Armadillo matrix implementation #42

Closed schaerfo closed 3 years ago

schaerfo commented 3 years ago

In CAST, matrix calculations can be performed with either Eigen or Armadillo through the common scon::mathmatrix interface. Of the two, Eigen is currently better supported (for example, the unit tests do not compile when using Armadillo), however, Armadillo has better performance since it is a wrapper around Lapack.

Since Eigen can also be configured to use Lapack/BLAS internally (see https://eigen.tuxfamily.org/dox/TopicUsingBlasLapack.html), I propose to assess the performance of this configuration in comparison to Armadillo using some suitable benchmark. Should it be satisfactory, the Armadillo implementation can be discarded, simplifying both the code base and the build system.

mrnicegyu11 commented 3 years ago

In my entropy project (not in devel/main branch) I use armadillo-specific code, specfically a gaussian-mixture-modell-fixing procedure. I am fine with purging armadillo from devel/main branch in any case. My dev branch needs to be maintained with armadillo as long as my paper draft remains unpublished. I would even be ok with keeping it "out of git" in a folder as a legacy code dump.

@schaerfo

mrnicegyu11 commented 3 years ago

Please inform me ahead of time in case you go ahead and commit breaking changes @schaerfo thx

schaerfo commented 3 years ago

In my entropy project (not in devel/main branch) I use armadillo-specific code, specfically a gaussian-mixture-modell-fixing procedure. OK, then we leave it in. I do not think forking CAST irreversibly would be a good idea.