Relative stopping criterion
Regarding #1051 this would fix the edge case with an rhs vector filled with zeroes.
Used '<=' instead of '<' in residual operations. Test cases will be added
Codecov Report
Patch coverage is 100.00% of modified lines.
:exclamation: Current head a9c1495 differs from pull request most recent head aa6ef60. Consider uploading reports for the commit aa6ef60 to get more accurate results
| Files Changed | Coverage |
|---|---|
| omp/stop/residual_norm_kernels.cpp | ø |
| reference/stop/residual_norm_kernels.cpp | 100.00% |
:loudspeaker: Thoughts on this report? Let us know!.
@keldu what's the status of this pull request? It seems to be good for review.
Quick question, this currently stands +4000 LOC changes and 174 files changed, which seems to be to much for fixing an edge case. Could it be that it is not based on dev?
NVM, I thought this would be ready for review.
So. I'm not fully sure about the tests. I did a slightly small one, but I am wholly unsure if it even works. What's missing anyway for now where I am stumped. Since I keep coming back without gaining any ground.
- [ ] Check all comments for the res_norm < \tau since we now do res_norm <= \tau
- [ ] Check test and add proper test.
And I misclicked. It is not ready for review :facepalm:
The most enlightening parts are the examples. And those basically only contain solver examples.
Quick question, this currently stands +4000 LOC changes and 174 files changed, which seems to be to much for fixing an edge case. Could it be that it is not based on
dev?
Yeah. I rebase'd on an incorrect develop branch. Caused a temporary issue which I didn't realise in time. I guess rebasing on origin/develop is the fix for that.
Kudos, SonarCloud Quality Gate passed! 
0 Bugs
0 Vulnerabilities
0 Security Hotspots
1 Code Smell
100.0% Coverage
11.1% Duplication
The version of Java (11.0.3) you have used to run this analysis is deprecated and we will stop accepting it soon. Please update to at least Java 17.
Read more here
I'm pretty sure my test is wrong btw. The problem mostly lies with the fact that I don't know what some of the functions do. And the examples don't cover initalizing already solved systems by hand.
Miss the implicit residual test. I think one of abs/rel should be enough? because they all go through the same kernel and the baseline calculation should be also tested in the existing tests.
I've added a rel criterion. Still not sure about my previous comment.
@keldu A simple way to check the test does what it's supposed to to is to revert the other changes and only leave the new test in, it should fail.
format!
LGTM. Some compiling issues and the tests need fix. Could you also rearrange the commit history by combining commits with the purpose? Also formatting in the end. I do not change my reviews status yet until the tests are correct.
I mean I could generally squash them? It's not a big change, so I would prefer to do it that way.
format!
format!
the first commit adds the tests which are failed in pipeline https://gitlab.com/ginkgo-project/ginkgo-public-ci/-/pipelines/1059541223 because it requires <= not <. adding <= and updating the doc are in the following commit.
format!
