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

Adding MGS H-GMRES(m) resolves #325 #326

Open
wants to merge 11 commits into
base: main
Choose a base branch
from
Open

Adding MGS H-GMRES(m) resolves #325 #326

wants to merge 11 commits into from

Conversation

pescap
Copy link

@pescap pescap commented Apr 3, 2022

No description provided.

@pescap
Copy link
Author

pescap commented Apr 4, 2022

#325

@pescap
Copy link
Author

pescap commented Apr 21, 2022

Dear all, do you have any suggestion concerning this PR? Thank you!

@lukeolson
Copy link
Member

@pescap Thanks for the PR! The preference is to keep the PR minimal -- so few formatting changes and consistent linting (I enabled CI including linting for this PR just now).

I can take a go at thinning this and also adding H to some of the parameterized tests...

@codecov-commenter
Copy link

codecov-commenter commented Apr 21, 2022

Codecov Report

Merging #326 (2c9c117) into main (af81d67) will decrease coverage by 0.00%.
The diff coverage is 76.92%.

@@            Coverage Diff             @@
##             main     #326      +/-   ##
==========================================
- Coverage   64.97%   64.96%   -0.01%     
==========================================
  Files          44       44              
  Lines        5524     5529       +5     
  Branches     1264     1265       +1     
==========================================
+ Hits         3589     3592       +3     
- Misses       1435     1436       +1     
- Partials      500      501       +1     
Impacted Files Coverage Δ
pyamg/krylov/_gmres.py 50.00% <0.00%> (-12.50%) ⬇️
pyamg/krylov/_gmres_mgs.py 74.16% <90.90%> (+0.66%) ⬆️

📣 Codecov can now indicate which changes are the most critical in Pull Requests. Learn more

@lukeolson
Copy link
Member

lukeolson commented Apr 21, 2022

@pescap what do you think about renaming H? In terms of code readability I'd like to keep H for the upper Hessenbergs in order to match the typical notation (from Saad, van der Vorst, and others). Looking at the papers you link in #325 it doesn't appear that there's any consistent notation for the weight. But D might make sense? Or W (for weight)?

@pescap
Copy link
Author

pescap commented Apr 21, 2022

@pescap what do you think about renaming H? In terms of code readability I'd like to keep H for the upper Hessenbergs in order to match the typical notation (from Saad, van der Vorst, and others). Looking at the papers you link in #325 it doesn't appear that there's any consistent notation for the weight. But D might make sense? Or W (for weight)?

Sounds good! I recommend using D.

@pescap
Copy link
Author

pescap commented Apr 25, 2022

Hi @lukeolson, let me know if you have further questions. As soon as the PR is merged, I plan to create an issue to discuss and compare l2- and H^1-based GMRES for a real AMG case! Best wishes,
Paul

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.

None yet

3 participants