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

linalg eig: generalized eigenvalue problem #909

Merged
merged 16 commits into from
Jan 5, 2025

Conversation

perazz
Copy link
Contributor

@perazz perazz commented Dec 13, 2024

This PR proposes to address #907: Introduce the generalized eigenvalue option, A⋅x=λ⋅B⋅x.

Proposed interface

  • call eig(a, b, lambda [, right] [, left] [, overwrite_a] [, overwrite_b] [, err])
  • lambda = eigvals(a, b [, err])

For comparison, the standard problem is solved with

  • call eig(a, lambda [, right] [, left] [, overwrite_a] [, err])
  • lambda = eigvals(a [, err])

Key facts

  • generalized problem uses *GGEV LAPACK backend
  • choice between standard vs. generalized problem is made via an interface, so the appropriate backend can be called. (b is not an optional argument)
  • Eigenvalue scaling is performed according to the same SciPy logic

Prior art

  • SciPy: eig(a, b=None, left=False, right=True, overwrite_a=False, overwrite_b=False, check_finite=True, homogeneous_eigvals=False)

cc: @jvdp1 @jalvesz @fortran-lang/stdlib

doc/specs/stdlib_linalg.md Outdated Show resolved Hide resolved
@perazz perazz marked this pull request as ready for review December 13, 2024 21:14
Copy link
Member

@jvdp1 jvdp1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you @perazz . LGTM. It is a nice extension of eig.

doc/specs/stdlib_linalg.md Outdated Show resolved Hide resolved
doc/specs/stdlib_linalg.md Outdated Show resolved Hide resolved
doc/specs/stdlib_linalg.md Outdated Show resolved Hide resolved
example/linalg/example_eig_generalized.f90 Outdated Show resolved Hide resolved
example/linalg/example_eigvals_generalized.f90 Outdated Show resolved Hide resolved
src/stdlib_linalg.fypp Show resolved Hide resolved
src/stdlib_linalg_eigenvalues.fypp Show resolved Hide resolved
src/stdlib_linalg_eigenvalues.fypp Show resolved Hide resolved
@perazz
Copy link
Contributor Author

perazz commented Dec 21, 2024

Thank you @jvdp1 for the review and for catching the docs typo. It is much more clean now.

PS I notice that sometimes loadtxt still fails in the CI. Now we have an error message, it says:
loadtxt: error <end-of-file during read, unit -129, file /home/runner/work/stdlib/stdlib/example/io/example.dat> reading line 1 of example.dat. (Intel compiler)

@jvdp1
Copy link
Member

jvdp1 commented Dec 21, 2024

I notice that sometimes loadtxt still fails in the CI. Now we have an error message, it says:
loadtxt: error <end-of-file during read, unit -129, file /home/runner/work/stdlib/stdlib/example/io/example.dat> reading line 1 of example.dat.

I have a similar issue in my prodcution code. I avoid it by compiling different codes for different compilers. Quite annoying... And I don't know if it is a bug of if the standard does not specifiy this behaviour.

@perazz
Copy link
Contributor Author

perazz commented Dec 26, 2024

By looking at the code, I really see no issues both in the default and the specified format, so I'm tempted to think of a bug. Maybe the only potential problem comes from non-advancing I/O in number_of_columns? To answer that, I implemented a better error message 8297763, hopefully that will help us understand.

@jvdp1
Copy link
Member

jvdp1 commented Dec 26, 2024

By looking at the code, I really see no issues both in the default and the specified format, so I'm tempted to think of a bug. Maybe the only potential problem comes from non-advancing I/O in number_of_columns? To answer that, I implemented a better error message 8297763, hopefully that will help us understand.

Thank you. Indeed, hopefully we can understand it soon.

Copy link
Contributor

@jalvesz jalvesz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM @perazz! Great addition!! I read through the PR and it really looks neat.

@perazz
Copy link
Contributor Author

perazz commented Jan 3, 2025

Thanks again @jvdp1 @jalvesz for the reviews, let's wait another couple of days and then merge, if there are no further comments.

@perazz perazz merged commit b5b86a7 into fortran-lang:master Jan 5, 2025
15 checks passed
@perazz perazz deleted the generalized_eig branch January 5, 2025 13:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants