added a default constructor

[jewettaij]

Changes

In the current version, the user must pass lambda expression to the mv_mul argument of the constructor. Is this necessary? Is it okay if I add a default constructor? This pull request includes a constructor that requires no arguments (LambdaLanczos::LambdaLanczos()).

I also added some "Set()" functions (but these are not important to me).

Summary

I added this because am trying to define a new class (eg PEigenCalculator) which contains a LambdaLanczos as a data member (ll_engine). I created this wrapper so that people who don't like lamda-expressions can use "PEigenCalculator" instead of LambdaLanczos. Unfortunately, I could not figure out how to write the constructor for PEigenCalculator. I added a default constructor (LambdaLanczos::LambdaLanczos()) to make it easier to write PEigenCalculator::PEigenCalculator().

Details (feel free to skip)

When initializing the "ll_engine" member, I could not figure out how to supply the lambda expression that LambdaLanczos() contructor needs as an argument. So I created a constructor for LambdaLanczor that requires no arguments.

Is this a bad idea? Is there another way?
(I admit I am not very familiar with lambda expressions in C++11.)

More Details:

The user must eventually specify the size of the matrix, and the matrix before invoking run(). I added an assert(matrix_size > 0); statement at the beginning of LambdaLanczos::run() to guarantee this.

Example code:

template<typename Scalar>
class PEigenCalculator {
  size_t n;                  // the size of the matrix
  vector<vector<Scalar> > M; // the matrix
  vector<Scalar> evec;       // preallocated vector (needed by lambda_lanzcos)
  LambdaLanczos<Scalar> ll_engine;  // this is the object that does the work
  void Init();
public:
  /// constructor
  PEigenCalculator(int n=0, bool findmax = false):ll_engine() {
    Init(n, findmax);
  }

  /// @brief  Calculate the principal eigenvalue and eigenvector of a matrix.
  /// @return Return the principal eigenvalue of the matrix.
  ///         If you want the eigenvector, pass a non-null "evec" argument.
  Scalar
  PrincipalEigen(Scalar const* const *matrix,  //!< the input patrix
                 Scalar *eigenvector=nullptr);   //!< optional: store eigenvector
}; // class PEigenCalculator

(For completeness, I will attach the "peigencalc.hpp" file which defines "PEigenCalculator". Feel free to ignore this file.)

Get() and Set() commands?

Do you think it is a good idea to make the data members private?

template <typename T>
class LambdaLanczos {
public:
  // ...
  int matrix_size;
  int max_iteration;
  real_t<T> eps = minimum_effective_decimal<real_t<T>>() * 1e3;
  real_t<T> tridiag_eps_ratio = 1e-1;
  int initial_vector_size = 200;
  bool find_maximum = false;
  real_t<T> eigenvalue_offset = 0.0;
  //...
}

I also added some "Set()" commands to allow users to modify the data members of LambdaLanczos. (Perhaps these are not necessary.) I did not add "Get()" functions yet.

Thanks again for creating lambda-lanczos

-Andrew

[jewettaij]

Example (feel free to ignore)

If you are curious, I attached a directory containing the "peigencalc.hpp" file. This file defines the "PEigenCalculator" class which an example of the wrapper I am trying to write. I included a modified version of the "lambda_lanczos_test.cpp" which uses PEigenCalculator instead of LambdaLanczos in "test1()". (I just wanted to demonstrate that PEigenCalculator works. I am not suggesting we should modify the "lambda_lanczos_test.cpp" file already included with this repository.)

Please do not feel obligated to read my code (or implement my suggestions). If you do not approve of my pull request, I can still make the changes I need to my version of your code.
-Andrew

new_files_2019-11-27.tar.gz

[mrcdr]

Thank you for your comment. In my opinion, I want my code to stay the same.

About default constructor

I designed LambdaLanczos as a "disposable object", whose size is very small, so its creation and deletion cost is reasonable. In addition, the cost to create functional object (i.e. lambda object) is also reasonable (see this Stack overflow article).
Especially the costs are far smaller than the cost of diagonalization with a large matrix which requires Lanczos-like algorithm.
So I don't recommend to make LambdaLanczos be contained in some object as a class member.
I attach a code edited from your one to show what I intended. Please see it (added comments are tagged with "@mrcdr"). As long as the LambdaLanczos is used as a disposable object, I think the situation that the default constructor is needed wouldn't be likely to happen.
peigencalc_mrcdr.zip

About Set() and Get()

The Set()/Get() implementation might be controversial. Traditional languages like Java have taken the strategy to make member variables private and define corresponding Set()/Get() for a long time. However, some modern languages like Kotlin (my favorite one) avoid the use of Set()/Get() (
Googling with words "getter setter evil" shows some claims why they shouldn't be used).
Especially in my case, the every member variables are readable and writable so the use of Set()/Get() is completely the same as just a member variable access.
I think the only possibility that Set() should be used is range checking; if some variable which should be positive is about to be set to negative, throw an exception.

Of course it's free and welcome to edit LambdaLanczos and make it suitable for your work!

Thank you,

[jewettaij]

Thank you for your comment. In my opinion, I want my code to stay the same.

About default constructor

I designed LambdaLanczos as a "disposable object", whose size is very small, so its creation and deletion cost is reasonable. In addition, the cost to create functional object (i.e. lambda object) is also reasonable (see this Stack overflow article).

Thank you. That link was useful. I do not know much about lambda objects.

Especially the costs are far smaller than the cost of diagonalization with a large matrix which requires Lanczos-like algorithm.

I'm embarrassed to admit that I admit that I only use LambdaLanczos to compute eigenvectors of a 4x4 matrix. I planned to use "Eigen", but the other LAMMPS developers did not like the fact that it uses a more complicated license (the MPL2). (It's also far too big. Your files are nice and small.)

So I don't recommend to make LambdaLanczos be contained in some object as a class member.

Actually I did create a PEigenCalculator class. But what I did does not require any modification to "lambda_lanczos.hpp". (This is because the lambda object is not created until the eigenvalue and eigenvector needs to be calculated. I followed your example.)

(If you're curious, my most recent version is here. I am not certain that the LAMMPS developers will let me use your "lambda_lanczos.hpp" library, so I want to wrap it with a class like PEigenCalculator that I can replace with something else later if I have to. Most eigenvalue calculators have some internal data that needs to be allocated on the heap ahead of time, such as temporary matrices and vectors.)

I attach a code edited from your one to show what I intended. Please see it (added comments are tagged with "@mrcdr"). As long as the LambdaLanczos is used as a disposable object, I think the situation that the default constructor is needed wouldn't be likely to happen.
peigencalc_mrcdr.zip

Your modified code was helpful to me. Thank you.

About Set() and Get()

The Set()/Get() implementation might be controversial. Traditional languages like Java have taken the strategy to make member variables private and define corresponding Set()/Get() for a long time. However, some modern languages like Kotlin (my favorite one) avoid the use of Set()/Get() (
Googling with words "getter setter evil" shows some claims why they shouldn't be used).

I was guilty of that kind of evil. (I put too many "Set" functions in your code.)

Especially in my case, the every member variables are readable and writable so the use of Set()/Get() is completely the same as just a member variable access.
I think the only possibility that Set() should be used is range checking; if some variable which should be positive is about to be set to negative, throw an exception.

Of course it's free and welcome to edit LambdaLanczos and make it suitable for your work!

Thank you,

Thank you. I will let you know if I run into other small issues.