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

Increase gram_schmidt default reiteration_tol to 9e-1 #1154

Merged
merged 1 commit into from Nov 17, 2020
Merged

Conversation

sdrave
Copy link
Member

@sdrave sdrave commented Nov 12, 2020

See discussion in #1067

Fixes #1067.

@sdrave sdrave added the pr:change Change in existing functionality label Nov 12, 2020
@sdrave sdrave added this to the 2020.2 milestone Nov 12, 2020
@sdrave
Copy link
Member Author

sdrave commented Nov 12, 2020

@lbalicki, the CI build for this PR should fail with test failures for the samdp algorithm. Can you take a look at what is happening?

@sdrave sdrave requested a review from lbalicki November 12, 2020 16:51
@lbalicki
Copy link
Member

There seems to be an Inversion error (RuntimeError: Factor is exactly singular) when executing one of the samdp tests. It only happens on the pip_installed test and unfortunately I was not able to recreate this type of behaviour on my machine. Is it possible to either get the data of the CI test or recreate the pip_installed test pipeline on my machine?

Looking at the CI output I can not really tell what the problem is. Especially the inversion error occurs when calling tEmA.apply_inverse_adjoint(...) and one line before that the line tEmA.apply_inverse(...) is executed succesfully. How can the numpy operator tEmA be invertible during the apply_inverse function call and its adjoint be singular in the apply_inverse_adjoint call?

@sdrave
Copy link
Member Author

sdrave commented Nov 13, 2020

There seems to be an Inversion error (RuntimeError: Factor is exactly singular) when executing one of the samdp tests. It only happens on the pip_installed test and unfortunately I was not able to recreate this type of behaviour on my machine.

That's rather strange. @renefritze, do you have an idea where this comes from?

I can reproduce the failure on my system. I have attached the non-invertible operator and backtrace. (Load the operator using pickle.) The parameters for the test were test_samdp[False-LR-15-2-2-50].

backtrace.txt operator.dat.gz

How can the numpy operator tEmA be invertible during the apply_inverse function call and its adjoint be singular in the apply_inverse_adjoint call?

I guess both operators will be singular, but the linear solver doesn't notice in one case (even for direct solvers, floating-point arithmetic may make the operator appear non-singular).

@renefritze
Copy link
Member

The PR build is fine btw. All pip installed * there ran on openstack machines, in the push build all pip installed * failed on our local machine.

@lbalicki
Copy link
Member

I think I found the issue. If an exact eigenvalue theta of (A, E) is passed to the _twosided_rqi method, the linear combination of operators theta * E - A will become singular. With #1156 the check if theta is an eigenvalue is improved, so it will hopefully take the case into account where the test fails here.

@codecov
Copy link

codecov bot commented Nov 17, 2020

Codecov Report

Merging #1154 (3a89ce4) into master (c602a0a) will decrease coverage by 0.01%.
The diff coverage is n/a.

Impacted Files Coverage Δ
src/pymor/algorithms/gram_schmidt.py 97.93% <ø> (ø)
src/pymor/bindings/fenics.py 84.30% <0.00%> (-0.80%) ⬇️
src/pymor/vectorarrays/numpy.py 84.29% <0.00%> (-0.25%) ⬇️
src/pymor/vectorarrays/list.py 82.18% <0.00%> (ø)
src/pymor/bindings/ngsolve.py 84.61% <0.00%> (+0.76%) ⬆️

@sdrave
Copy link
Member Author

sdrave commented Nov 17, 2020

@lbalicki, good to merge?

@sdrave sdrave merged commit 728c040 into master Nov 17, 2020
2 checks passed
@sdrave sdrave deleted the reiteration_tol branch November 17, 2020 13:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pr:change Change in existing functionality
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Orthonormalization Issues in eigs
3 participants