Closed EduardFedorenkov closed 3 months ago
langou
commented
4 months ago Hi @EduardFedorenkov, That's great. Thanks for the work
I think that @jprhyne already started a similar work, so I think it would be good to get in synch. @jprhyne: can you PR your work so that we compare the two PRs?
jprhyne
commented
4 months ago Yes, I just made one.
I have similar issues, and currently am looking at making a small test suite to more easily identify the errors.
EduardFedorenkov
commented
4 months ago Looks like I fix the bug. Thanks to @jprhyne PR #1020. @jprhyne handled the case LASTV = 1 correctly, unlike me!
All netlib tests and my tests - passed.
jprhyne
commented
4 months ago Awesome! I'll look at my implementation later today and hopefully fix the issue as well. I'll look at the difference between what we did and hopefully it'll be more obvious then!
@EduardFedorenkov since you wanted to work together, do you want to go ahead and do d/z and I do c/s separately?
Edit: I just needed to check that LASTV option is 1 not if m and n are 1.
Edit2: Updated question to reflect my current plan.
EduardFedorenkov
commented
4 months ago Yes, I will take a look at the difference between my and your implementation. Then I think we need to align our versions. I think it is necessary to have almost the same code in {Z,D} and {C,S}. After this it was easy to finish the issue. If you already have some suggestions about the algorithm, please write it in this PR.
jprhyne
commented
4 months ago Hello Eduard,
I was thinking that since we are only adding the assumption that v(1) = 1 [or v(lastv)=1], that the general structure could stay similar. I do like the blocking idea on C, but I'm not sure I follow why we need to break it up in 4 sections.
Instead, basically do something like C = [ C1 C2 ] since that reflects that we are breaking up v as
v = [1 x ... x ]^T (potentially trailing zeros)
Maybe I'm misunderstanding something that is explained by the 4 block style.
Edit: above is the CH case. And for the HC case it would be something like C= [C1 C2]^T
jprhyne
commented
4 months ago Hey @EduardFedorenkov, I've merged differences aside from the ones I asked about above, I was wondering if this was an acceptable base for us to start with and then move on to the other precisions or if there were other comments or another preference.
Also, I do not mean to post many comments, just wanted to separate the thoughts into two separate blocks!
EduardFedorenkov
commented
4 months ago Hi, Johnathan!
You did the great job! I think now your implementation is totally correct and better than mine! I checked your realization on my bunch of tests, and it also passed them.
I have some comments about "clean code", but it is OK for now. Let's proceed with your realization. I will align my version with yours soon. You did D-precision, so you can proceed with Z.
If you share my opinion, then I would remove my implementation of D and do {C, S}.
codecov[bot]
commented
3 months ago Attention: Patch coverage is 0% with 283 lines in your changes missing coverage. Please review.
Project coverage is 0.00%. Comparing base (
7295ac1) to head (690067c). Report is 25 commits behind head on master.:exclamation: Current head 690067c differs from pull request most recent head c8b1a51
Please upload reports for the commit c8b1a51 to get more accurate results.
| Files | Patch % | Lines |
|---|---|---|
| SRC/clarf1f.f | 0.00% | 35 Missing :warning: |
| SRC/clarf1l.f | 0.00% | 33 Missing :warning: |
| SRC/slarf1f.f | 0.00% | 33 Missing :warning: |
| SRC/slarf1l.f | 0.00% | 31 Missing :warning: |
| SRC/cunbdb.f | 0.00% | 22 Missing :warning: |
| SRC/sorbdb.f | 0.00% | 22 Missing :warning: |
| SRC/cunbdb4.f | 0.00% | 9 Missing :warning: |
| SRC/sorbdb4.f | 0.00% | 9 Missing :warning: |
| SRC/cunbdb2.f | 0.00% | 5 Missing :warning: |
| SRC/cunbdb3.f | 0.00% | 5 Missing :warning: |
| ... and 43 more |
:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.
Hi, @langou!
I develop the first version of DLARF1F. Some netlib tests for QR are failed. Looks like errors in some corner cases. Please take a look. I will send some description about idea of the algorithm later.