Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
Tests for multiply (R←X×Y) #76
base: main
Are you sure you want to change the base?
Tests for multiply (R←X×Y) #76
Changes from 22 commits
ee2d56c
1cb5f8c
47f8c82
97cbf4c
8bfb44e
508ad33
21de819
9ee6844
8ab3d3b
fa62970
3419e63
d87b2f5
710cd5a
9983784
acb2ce1
847d7e8
788aeac
cae4236
ce26d8f
f05e0b1
2151fb8
ff26dd2
66cc49c
0045eec
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you use {*(⍟⍺)+⍟⍵} as the model (using logarithms instead)? This would remove the need to handle complex numbers specifically.
Or, alternatively, could you use multiple models for redundancy?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is a minor problem with the model that it gives complex answers from negative numbers. I changed the model to:
(it is a makeshift approach, i know the × should not be there to multiply the sign)
The problem with this now is the direction operator approach will not give me the sign of the for the complex numbers but the direction of the complex vector(i think). Is there a solution to this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Again, I would be testing more "interesting" values too. For mantis 21743 the test was using 9007199254740992, which was the largest float that can be expressed as an integer.
Perhaps also use inputs that you know will produce (close to) the largest resulting float answer (~1e154)?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The more bigger values are supposed to be tested by the random number generator in
RunVariations
but because of the limitations of the current model I had turned it off. I will move to your model and try it out.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there a way I can easily find these 'interesting values' in mantis? For now, are there any specific values you would want to include other than this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Again, in general, testing more small numbers would be good. Including denormal values (e.g. 1e¯310). Not that it should cause any issues here, but still.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Multiply is a trivial primitive, but I'd still like to see tests for more places where we expect failure. E.g., ×⍨1e155. We don't want this to generate a 1287 accidentally, for example (even though that's improbable, we should still treat the interpreter as a black box and create that test).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Want to confirm:
Should I test if this gives a domain error or not?