You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@systemds.apache.org by GitBox <gi...@apache.org> on 2020/07/10 23:29:38 UTC
[GitHub] [systemds] skogler opened a new pull request #987: Check for extra values in test matrix comparison.
skogler opened a new pull request #987:
URL: https://github.com/apache/systemds/pull/987
In the case of expected matrices with zero values, TestUtils was not
correctly checking whether the actual test output matrix does have
values in these places.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [systemds] Baunsgaard commented on pull request #987: Check for extra values in test matrix comparison.
Posted by GitBox <gi...@apache.org>.
Baunsgaard commented on pull request #987:
URL: https://github.com/apache/systemds/pull/987#issuecomment-657127885
Hi @skogler,
I see that this is a valid point and something we need !
Could it maybe make sense to compare the lengths of the HashMaps for better performance?
Since if the number differs then the output is anyway incorrect.
Best regards
Sebastian
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [systemds] Baunsgaard commented on pull request #987: Check for extra values in test matrix comparison.
Posted by GitBox <gi...@apache.org>.
Baunsgaard commented on pull request #987:
URL: https://github.com/apache/systemds/pull/987#issuecomment-658120203
Hi @skogler,
Thanks for the PR! :+1: .
I ended up changing it slightly to not unnecessarily increase testing time. the most common case is that the output is correct and the sizes are the same, therefore the edge case where the number of values are different the test fails, while also reporting the incorrect indexes.
The only case that does not report all the incorrect indexes is if the actual and expected contains the same number of elements, but then the test would fail now, since the indexes would have to be different, or the values contained different.
```java
allKeys.addAll(expectedValues.keySet());
if(expectedValues.size() != actualValues.size())
allKeys.addAll(actualValues.keySet());
```
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [systemds] Baunsgaard closed pull request #987: Check for extra values in test matrix comparison.
Posted by GitBox <gi...@apache.org>.
Baunsgaard closed pull request #987:
URL: https://github.com/apache/systemds/pull/987
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [systemds] skogler commented on pull request #987: Check for extra values in test matrix comparison.
Posted by GitBox <gi...@apache.org>.
skogler commented on pull request #987:
URL: https://github.com/apache/systemds/pull/987#issuecomment-657750732
Hi, yeah sure if its needed I can add it. But then you would not get the
output showing which elements are incorrect. For unit tests I would prefer
getting it, but I don't know all the test cases, can't say if it would be a
problem or not.
On Sat, 11 Jul 2020, 22:52 Sebastian Baunsgaard, <no...@github.com>
wrote:
> Hi @skogler <https://github.com/skogler>,
>
> I see that this is a valid point and something we need !
> Could it maybe make sense to compare the lengths of the HashMaps for
> better performance?
> Since if the number differs then the output is anyway incorrect.
>
> Best regards
> Sebastian
>
> —
> You are receiving this because you were mentioned.
> Reply to this email directly, view it on GitHub
> <https://github.com/apache/systemds/pull/987#issuecomment-657127885>, or
> unsubscribe
> <https://github.com/notifications/unsubscribe-auth/AAH4BVOZYZFJ4WPEYAJBQ6TR3DGI7ANCNFSM4OXAIB5A>
> .
>
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org