fix is_symmetric for complex PSD/NSD matrices #200
Conversation
…dentityLinearOperator to avoid test failures
|
Can confirm only one failing test now |
patrick-kidger
left a comment
patrick-kidger
left a comment
There was a problem hiding this comment.
One comment only, LGTM!
| def _has_real_dtype(operator) -> bool: | ||
| """Check if all dtypes in an operator's structure are real (not complex).""" | ||
| leaves = jtu.tree_leaves(operator.in_structure()) | ||
| return all(jnp.issubdtype(leaf.dtype, jnp.floating) for leaf in leaves) |
There was a problem hiding this comment.
We should probably check the out_structure as well? (Even if the only codepath at the moment is for a posdef/negdef operator, for which the new check ensure this is already true.)
There was a problem hiding this comment.
Also this seems like it might produce a silent flake if e.g. an integer dtype somehow slips through our floating-point-casting elsewhere. Perhaps we should explicitly check for floating, complex, and if neither then raise an error?
|
Okay, deciding to go ahead and merge as this is blocking other stuff I'm working on :D (I'll incorporate my comment into #203.) |
|
Ah, sorry, i actually addressed your comment and committed but I must have tried to push to your repo rather than my fork and didn't notice the error message. 🤦♂️ Will rebase my other PRs on yours when that's merged. Thanks! |
2 participants
Currently, is_symmetric returns True for PSD/NSD matrices. This led to failing test in test_well_posed.py when we changed our transpose rules, this should be a minimal fix.
Note we could go all out and add
is_hermitiantests and.Hhelper function for less verbosity, but as I think we're not really prioritising complex support right now (e..g see comment in CG solver) that can be delayed for now.