Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Discussion on future development of Spectra #92

Open
yixuan opened this issue May 19, 2020 · 16 comments
Open

Discussion on future development of Spectra #92

yixuan opened this issue May 19, 2020 · 16 comments

Comments

@yixuan
Copy link
Owner

yixuan commented May 19, 2020

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 SelectionRule template parameter #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 Adding Davidson and Jacobi Davidson #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.

@jschueller
Copy link
Contributor

jschueller commented May 19, 2020

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.

@JensWehner
Copy link
Contributor

  • 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.

@zhaiyusci
Copy link

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.)

@yixuan
Copy link
Owner Author

yixuan commented May 20, 2020

@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
Copy link
Owner Author

yixuan commented Jun 4, 2020

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.

@jschueller
Copy link
Contributor

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

@yixuan
Copy link
Owner Author

yixuan commented Jun 5, 2020

@jschueller Sure. It has been added: a97bf20.

@jschueller
Copy link
Contributor

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
Copy link
Contributor

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 ?

@yixuan
Copy link
Owner Author

yixuan commented Jun 7, 2020

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.

@jschueller
Copy link
Contributor

hello, any news on the 1.y release ?

@yixuan
Copy link
Owner Author

yixuan commented Jan 10, 2021

@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;
}

This was referenced Jan 10, 2021
@yixuan
Copy link
Owner Author

yixuan commented Jan 11, 2021

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.

@jschueller
Copy link
Contributor

cool, thanks

@Enlil50
Copy link

Enlil50 commented Nov 2, 2022

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?

@peekxc
Copy link

peekxc commented Nov 7, 2022

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants