Break Up Moore Penrose Test Into Smaller Tests For Clarity and Ease of Debugging #284
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…efinition in the wiki more closely.
…c failure happens more precisely.
… ones to aid debugging of the errratic test.
|
@olekscode @SergeStinckwich We may have discovered the source of the problem:
I am unsure of the purpose of this test to be honest, and I propose removing the loop and changing the purpose of the test. |
hemalvarambhia
commented
Jun 29, 2022
| a := PMMatrix rows: 3 columns: 3 random: 1.0. | ||
| self assert: (a mpInverse closeTo: a inverse). | ||
| a := PMSymmetricMatrix new: 4 random: 1.0. | ||
| self assert: (a mpInverse closeTo: a inverse) ] |
There was a problem hiding this comment.
I am seeking a mathematician to help me in understand the needs of this test.
There was a problem hiding this comment.
It appears, after splitting the testMoorePenroseInverse test up, this part was the source of the random test failure. I infer from the repetition that this may be some kind of exhaustive data-driven test. I've changed this so as to get a more deterministic test.
hemalvarambhia
commented
Jun 30, 2022
| | identityMatrix | | ||
| "These two assertions are what define a pseudoinverse. They are known as | ||
| the Moore–Penrose conditions of which there are four, but here we have two. The other two | ||
| are that (A * A+) and A+ * A are Hermitian. |
There was a problem hiding this comment.
Asserting that A x A+ is Hermitian failed, sadly.
SergeStinckwich
approved these changes
Jun 30, 2022
| a := PMMatrix rows: 3 columns: 3 random: 1.0. | ||
| self assert: (a mpInverse closeTo: a inverse). | ||
| a := PMSymmetricMatrix new: 4 random: 1.0. | ||
| self assert: (a mpInverse closeTo: a inverse) ] |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Issue: #279
I have started by trying to improve clarity in the tests by breaking the test up into well named smaller ones. I hypothesise that it will pin point which scenario is failing.