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

Proposal: Add helper methods is_hermitian and is_unitary for the Matrix struct #1406

Open
haoxins opened this issue Jun 18, 2024 · 3 comments

Comments

@haoxins
Copy link

haoxins commented Jun 18, 2024

Hi,
I already have these extent methods in my own workspace,
so I wonder is it accepted to be added into the Nalgebra.

@Andlon
Copy link
Collaborator

Andlon commented Jun 21, 2024

I agree these methods could be useful. They're, however, quite difficult to get "right". In your code, you check for exact inequality. However, after one or more arithmetic operations, you'll almost certainly have some ever-so-slight errors. In this case, a matrix may not be technically Hermitian or unitary, but for all intents and purposes it should be considered so.

This implies that we should consider approximate comparison. That, however, opens up a number of design questions: What kind of approximate comparison is likely to match what a user expects? Should they provide a tolerance themselves?

This seemingly trivial feature quickly gets a lot more complex once you start to think about everything that can go wrong once inexact floating-point computations enter the scene. I think for us to move forward with this, we should:

  1. Have strong motivation for these methods. When would you need to use them? In most cases the user already knows if their matrix is supposed to be Hermitian or not, and when possible, this is by far preferable. In which cases does the user need to know if the matrix is Hermitian, but they do not know in advance?
  2. Consider prior work. How is this handled by other popular linear algebra libraries?

@haoxins
Copy link
Author

haoxins commented Jun 22, 2024

However, after one or more arithmetic operations, you'll almost certainly have some ever-so-slight errors.

Agreed totally. However, the two methods are simple wrappers of the existing methods so that these methods don't introduce more uncertainty.

However, after one or more arithmetic operations, you'll almost certainly have some ever-so-slight errors. In this case, a matrix may not be technically Hermitian or unitary, but for all intents and purposes it should be considered so.

We already have the eps for is_identity, but the eq does not. Is another equal method more suitable?

Have strong motivation for these methods.

You are right, there is no strong motivation but some convenience.
I personally use these as some assertions inside my code logic.

@Andlon
Copy link
Collaborator

Andlon commented Jun 30, 2024

You're right, there's already is_identity and is_orthogonal, both of which eventually boil down to approx::relative_eq!. I guess for consistency we could implement is_unary in the same way is_orthogonal is implemented.

is_hermitian is trickier though, because we don't have a reference for "scale" like we do for is_identity (e.g. compare to 1.0).

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

No branches or pull requests

2 participants