spectra icon indicating copy to clipboard operation
spectra copied to clipboard

Discussion on future development of Spectra

Open yixuan opened this issue 4 years ago • 16 comments

This thread is used to discuss the future development of Spectra. I would be glad to hear your voice about how this library would look like in the next few major versions.

Let me first outline what I have in mind so far:

  1. The first thing I want to do is to remove the SelectionRule template parameter from all eigen solvers, as suggested by @wo80 in #61. I don't see obvious disadvantage of this change, except that it breaks the current API, which I will discuss below. On the other hand, the benefit is clear: it makes wrappers of Spectra (including my own RSpectra R package) much cleaner, and allows you to use multiple selection rules in one program without compiling the whole solver many times.
  2. I tend to adopt newer C++ standards, e.g. C++11, in future development of Spectra. Honestly speaking I don't have much programming experience on the new standards, but recently I started to learn and expect that some features of C++11 can improve the quality of the code.
  3. At least some basic support for complex-valued matrices should be added to Spectra. This was requested many times in GitHub Issues.
  4. Other iterative eigen solvers may be added to Spectra also, for example Jacobi-Davidson, as suggested in #87. How to unify the API is something we need to figure out. @JensWehner

About backward compatibility: I believe this is a headache for virtually every open source library, but I tend to be "aggressive" on this aspect. Personally I treat Spectra mostly as a research project, and I want to quickly drop poor designs and outdated features. In fact I have already broken the API several times (changelog).

My plan is to maintain the current API at version 0.9.x, and develop all new features in 1.y.z+. One thing I'm not familiar with is the consequence to community-maintained packages such as Conda. Any thoughts on this? @guacke @jschueller

Other thoughts/suggestions? Please feel free to leave your comments below.

yixuan avatar May 19 '20 03:05 yixuan

About compatibility, I agree that it's best to drop bad apis, as spectra is header only it wont break binaries, also C++11 should be supported by all deployed compilers by now.

However I suggest to have version macros ie, `#define SPECTRA_VERSION 000900, see boost for example.

jschueller avatar May 19 '20 05:05 jschueller

  • I agree about C++11. All compilers support C++14 now, so 11 is a sure bet.
  • The Davidson API we should have a discussion. Do you mind having a chat? I can send you a microsoft teams invite.
  • I think one area spectra can still improve is better Testing/CI. We will probably add sth. there for the davidson stuff, if you would not mind. For example we can check if the code that is submitted is properly formatted.

JensWehner avatar May 19 '20 08:05 JensWehner

I agree about the C++ 11 thing, because it is the beginning of modern C++.

About breaking old API, what about mark some of them as deprecated and remove them step by step? (However, [[deprecated]] is since C++14.)

zhaiyusci avatar May 19 '20 15:05 zhaiyusci

@jschueller Thanks. I'll take a look.

@JensWehner Sure. We can discuss the details offline via email or other means. I may not be available to reply promptly as I have other duties this week.

@zhaiyusci Yeah, but the issue is that all API will be broken, if I do the first point mentioned above.

yixuan avatar May 20 '20 00:05 yixuan

Hi all, here are some updates.

I've created a 1.y.z branch as a preview of the planned changes in the next major version. A brief changelog can be found here, with the changes in example code, although the API documentation has not been updated.

yixuan avatar Jun 04 '20 21:06 yixuan

can you add a version macro if you change the api ?

jschueller avatar Jun 05 '20 11:06 jschueller

@jschueller Sure. It has been added: https://github.com/yixuan/spectra/commit/a97bf2034dc7e3b84862ae6a3aadf577e54724dc.

yixuan avatar Jun 05 '20 13:06 yixuan

About conda compatibility, packages on conda will likely need patching to continue to build, unless we decide to pin the version on conda-forge if spectra moves on too fast. Hopefully it wont affect too many packages, only openturns I think :)

jschueller avatar Jun 05 '20 19:06 jschueller

I just tested the 1y branch, and I could adapt to the new api in openturns without any trouble and pass the tests. I like the strongly typed enums, like CompInfo, but I'd like some function to convert it to string, (currently I use a static_cast to convert it to int and print that) Good job for the 1y branch, when do you plan to release it ?

jschueller avatar Jun 05 '20 19:06 jschueller

Ah, personally I'd like to further polish it a little bit and add some new features before the actual release. After all it is marked as "1.0.0" and I wish to stabilize the API. And that's why I created this thread and wanted to hear your comments.

yixuan avatar Jun 07 '20 14:06 yixuan

hello, any news on the 1.y release ?

jschueller avatar Jan 06 '21 10:01 jschueller

@jschueller I think it is close. I'm going to merge this branch and do some cleanups. There will be another big API change though: I want to remove the Scalar template parameter in eigen solvers, and let the matrix operator define a public typedef instead (built-in classes will do so). This should make the code a bit cleaner. An example would be as follows:

#include <Eigen/Core>
#include <Spectra/SymEigsSolver.h>
#include <iostream>

using namespace Spectra;

// M = diag(1, 2, ..., 10)
class MyDiagonalTen
{
public:
    using Scalar = double;  // This is now required for user-defined classes

    int rows() { return 10; }
    int cols() { return 10; }
    // y_out = M * x_in
    void perform_op(const double *x_in, double *y_out) const
    {
        for(int i = 0; i < rows(); i++)
        {
            y_out[i] = x_in[i] * (i + 1);
        }
    }
};

int main()
{
    MyDiagonalTen op;
    SymEigsSolver<MyDiagonalTen> eigs(op, 3, 6);
    eigs.init();
    eigs.compute(SortRule::LargestAlge);
    if(eigs.info() == CompInfo::Successful)
    {
        Eigen::VectorXd evalues = eigs.eigenvalues();
        std::cout << "Eigenvalues found:\n" << evalues << std::endl;
    }

    return 0;
}

yixuan avatar Jan 10 '21 00:01 yixuan

Hi All, I have drafted a migration guide to document the user-level changes from 0.9.0 to 1.0.0. Hope this makes the transition a little easier.

I'm going to marge the 1.y.z branch to master soon.

yixuan avatar Jan 11 '21 02:01 yixuan

cool, thanks

jschueller avatar Jan 11 '21 08:01 jschueller

Is there a way to implement a selection rule (or what it goes for it) to find an eigenvalue, the nearest to what I'm looking for, and x eigenvalues above or below it?

Enlil50 avatar Nov 02 '22 16:11 Enlil50

As an aside, I found the solver interfaces (such as SymEigsSolver) to be a little too restrictive for my use case. For example, there are use-cases where one may want to know the ritz values of the solver; maybe a Vector ritz_values(){...} would be useful to tack on.

peekxc avatar Nov 07 '22 19:11 peekxc