eigenvalues and eigenvectors using arb

[mmezzarobba]

The wrappers are marked as experimental because the corresponding arb functions are.

[mmezzarobba]

New commits:

​1d42e2b

#30393 wrap arb's experimental eigensolver

[slabbe]

Note that the signature of that method is changing in #29243. Maybe better to follow that?

[git]

Branch pushed to git repo; I updated commit sha1. New commits:

​e89ac63

#30393 match the new interface introduced in #29243

[mmezzarobba]

Replying to slabbe:

Note that the signature of that method is changing in #29243. Maybe better to follow that?

Thank you. I'm not setting that ticket as a dependency though, since these methods are experimental anyway.

[git]

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

​56f22b1

#30393 match the new interface introduced in #29243

[slabbe]

Few comments:

• "unable to certify the eigenvalues of this interval matrix", I would avoid the usage of the word interval which reminds of the CIF since the matrix belongs rather to the CBF.
• the other input should be documented as related to the generalized eigenvalue problem in the input block and/or in the raise NotImplementedError.

What should happen when the parent is RealBallField?

sage: M = matrix(3, [1, 1, 1, 1, 0, 0, 0, 1, 0])                                                       
sage: MRBF = M.change_ring(RBF)                                                                        
sage: MRBF.eigenvalues() 
KeyError: '_PolynomialRing_singular_repr__singular'
...
During handling of the above exception, another exception occurred:
...
AttributeError: 'PolynomialRing_field_with_category' object has no attribute '_PolynomialRing_singular_repr__singular'
...
During handling of the above exception, another exception occurred:
...
TypeError: no conversion of this ring to a Singular ring defined
...
During handling of the above exception, another exception occurred:
...
NotImplementedError: 

Can this benefits from your current branch?

[git]

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

​7f1b45e

#30393 match the new interface introduced in #29243

[mmezzarobba]

Replying to slabbe:

Few comments:

• "unable to certify the eigenvalues of this interval matrix", I would avoid the usage of the word interval which reminds of the CIF since the matrix belongs rather to the CBF.
• the other input should be documented as related to the generalized eigenvalue problem in the input block and/or in the raise NotImplementedError.

Done. (Though the second point is bugware. What we need IMHO is for each method's documentation to include or link to that of the methods it overrides, and to change the way we write docstrings to take advantage of that. It doesn't make any sense to repeat the same information over and over with no automatic way of keeping the various places in sync.)

What should happen when the parent is RealBallField?

[...]

Can this benefits from your current branch?

In principle yes (and matrices over RR and CC can benefit too), but at the moment we don't even have a dedicated class for matrices over RBF. I think we should keep that for another ticket, and probably wait until the methods are no longer marked experimental.

[git]

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

​5f6fb40

#30393 wrap arb's experimental eigensolver

[slabbe]

Replying to mmezzarobba:

Replying to slabbe: Done.

Thanks.

(Though the second point is bugware. What we need IMHO is for each method's documentation to include or link to that of the methods it overrides, and to change the way we write docstrings to take advantage of that. It doesn't make any sense to repeat the same information over and over with no automatic way of keeping the various places in sync.)

I agree with your point (out of sync). But, I do hate the documentation of G.plot(**kwds) which does not say which options are available.

That being said, I think my point in comment 2 was mostly to avoid using the extend option as a positional argument like:

sage: M = M.change_ring(CBF)
sage: M.eigenvalues(True)

since it will not be the 1st argument anymore after #29243. If you can force the extend to be a keyword argument like def eigenvalues(self, *, extend=False), then for me it is okay, even without the other argument. Do you mind to remove the other argument? Sorry for my slow convergence on clarifying my point of view.

I think we should keep that for another ticket, and probably wait until the methods are no longer marked experimental.

I agree.

[slabbe]

I get an error while building the documentation:

[dochtml] OSError: docstring of 
sage.matrix.matrix_complex_ball_dense.Matrix_complex_ball_dense.trace:11: 
WARNING: Duplicate explicit target name: "arb documentation".

In fact it seems to be triplicate:

+        See the `Arb documentation <http://arblib.org/acb_mat.html#c.acb_mat_eig_multiple>`_
+        for more information.
...
+        See the `Arb documentation <http://arblib.org/acb_mat.html#c.acb_mat_approx_eig_qr>`_
+        for more information.
...
+        See the `Arb documentation <http://arblib.org/acb_mat.html#c.acb_mat_eig_simple>`_
+        for more information.

[git]

Branch pushed to git repo; I updated commit sha1. New commits:

​edd1ae0

#30393 work around sphinx issue

[mmezzarobba]

Replying to slabbe:

If you can force the extend to be a keyword argument like def eigenvalues(self, *, extend=False), then for me it is okay, even without the other argument. Do you mind to remove the other argument?

It may still be slightly better to have it there, to be compatible with the generic method in case someone explicitly passes a second argument equal to None.

[mmezzarobba]

Replying to slabbe:

I get an error while building the documentation:

[dochtml] OSError: docstring of 
sage.matrix.matrix_complex_ball_dense.Matrix_complex_ball_dense.trace:11: 
WARNING: Duplicate explicit target name: "arb documentation".

I don't understand what is going on (why should two different inline links necessarily have distinct link texts?!), but let's see if the commit I just pushed solves the issue.

[slabbe]

Can you do the following change (x3) ?

-  See the <http://arblib.org/acb_mat.html#c.acb_mat_eig_multiple for
+  See http://arblib.org/acb_mat.html#c.acb_mat_eig_multiple for

[slabbe]

also:

- http://arblib.org/acb_mat.html#c.acb_mat_approx_eig_qr_
+ http://arblib.org/acb_mat.html#c.acb_mat_approx_eig_qr

[git]

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

​a39c8c6

#30393 work around sphinx issue

[mmezzarobba]

Oops. I went too fast indeed, thanks!

---

New commits:

​a39c8c6

#30393 work around sphinx issue

[slabbe]

I found the correct fix. In ReStructuredText syntax, one underscore at the end creates a target arb documentation which needs to be unique in the page while two underscores doesn't create such a target. See: ​https://stackoverflow.com/questions/27420317/restructured-text-rst-http-links-underscore-vs-use

-See the `Arb documentation <http://arblib.org/acb_mat.html#c.acb_mat_eig_multiple>`_
-See the `Arb documentation <http://arblib.org/acb_mat.html#c.acb_mat_approx_eig_qr>`_
-See the `Arb documentation <http://arblib.org/acb_mat.html#c.acb_mat_eig_simple>`_
+See the `Arb documentation <http://arblib.org/acb_mat.html#c.acb_mat_eig_multiple>`__
+See the `Arb documentation <http://arblib.org/acb_mat.html#c.acb_mat_approx_eig_qr>`__
+See the `Arb documentation <http://arblib.org/acb_mat.html#c.acb_mat_eig_simple>`__

Do you want to revert it back to using two underscores?

[git]

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

​b792cbd

#30393 fix external links in docstrings

[mmezzarobba]

Replying to slabbe:

I found the correct fix.

Oh, I didn't know about the difference, thank you!

Do you want to revert it back to using two underscores?

Done (but untested; I don't want to rebuild the doc right now).

[slabbe]

and I checked that documentation builds ok.

[slabbe]

Replying to mmezzarobba:

Replying to slabbe:

I found the correct fix.

Oh, I didn't know about the difference, thank you!

I created #30444 to add it to the documentation.

[gh-mwageringel]

While following the examples in the documentation in order to prepare something for the release tours, I noticed that the computed right eigenvectors are wrong. They do not satisfy the eigenvector equation. In fact, it seems to be a problem of transposition. The computed vectors are the rows of the eigenmatrix, but the eigenvectors would be the columns.

sage: from sage.matrix.benchmark import hilbert_matrix
sage: mat = hilbert_matrix(3).change_ring(CBF)
sage: [v[1][0].column() for v in mat.eigenvectors_right()]
⎡ ⎛ [0.827044926972 +/- 1.17e-13] + [+/- 1.08e-13]*I⎞  ⎛[0.459863904366 +/- 5.79e-13] + [+/- 1.22e-13]*I⎞
⎢ ⎜[-0.547448430721 +/- 4.33e-13] + [+/- 1.08e-13]*I⎟  ⎜[0.528290235067 +/- 5.58e-13] + [+/- 1.22e-13]*I⎟
⎣ ⎝[-0.127659329747 +/- 5.74e-13] + [+/- 1.08e-13]*I⎠, ⎝[0.713746885803 +/- 5.34e-13] + [+/- 1.22e-13]*I⎠,

 ⎛ [0.323298435244 +/- 6.18e-13] + [+/- 1.19e-13]*I⎞ ⎤
 ⎜ [0.649006658852 +/- 4.06e-13] + [+/- 1.19e-13]*I⎟ ⎥
 ⎝[-0.688671531671 +/- 4.92e-13] + [+/- 1.19e-13]*I⎠ ⎦

sage: mat.change_ring(CDF).eigenmatrix_right()[1]
⎛  0.8270449269720096  -0.5474484307206746 -0.12765932974653435⎞
⎜ 0.45986390436554375   0.5282902350674367    0.713746885803413⎟
⎝ 0.32329843524449886   0.6490066588517125  -0.6886715316713734⎠

[mmezzarobba]

Uh, thanks a lot for catching the issue! I thought I has checked...

[mmezzarobba]

Fixed (I hope) at #30568.

[slabbe]

Oups, I didn't notice that issue during the review. Thanks.

[slabbe]

Another follow up: #33652