Add abs() support for multivectors - #145
Conversation
Implement MultiVector.__abs__ as abs(self.norm().e) so abs(mv) returns a non-negative scalar, as specified in tBuLi#42.
|
Hi @HarshRajSinghania, thanks for the PR! The implementation looks good to me, but the tests seem to test several redundant things, and not the one thing that matters: can you add a test in e.g. Algebra(0,1) and test that the abs of a vector there is always positive? And can I also ask you to change the masking example in arrays.rst to use this new syntax? Thanks for your help. |
|
Thanks @tBuLi — addressed both review notes:
Pushed to this branch: |
| # abs() of a negative scalar should still be positive | ||
| s = vga2d.scalar(e=-2.5) | ||
| assert abs(s) == pytest.approx(2.5) | ||
| # comparison form requested in issue #42 |
There was a problem hiding this comment.
I do not get the two assertions here, because we already know that result == 5.0. So these next two lines don't add anything and should be removed.
There was a problem hiding this comment.
Removed the two redundant assertions after assert result == pytest.approx(5.0). The comparison cases and the Algebra(0, 1) positivity test are unchanged.
|
Removed the two redundant assertions in |
|
Thank you @HarshRajSinghania for your effort, I have now merged the PR! |
Summary
Implement
abs(mv)for kingdon multivectors.MultiVector.__abs__returnsabs(self.norm().e), a non-negative scalar coefficient.Motivation
Fixes #42. The issue requested that
abs()work on multivectors. The maintainer clarified that the expected result isabs(mv.norm().e)as a scalar so comparisons such aspoints[abs(lines) < 1]work, and that this should follownormrather than adding a new algebra operator.Implementation
MultiVector.__abs__next tonorm/normalizedinkingdon/multivector.py.abs()reusesnorm()and then takes the absolute value of the scalar part (.e).Testing
Added
test_abs_multivectorintests/test_kingdon.py:(3, -4)→abs(v) == 5abs(v) < 6, not< 1)Commands run locally (anywidget was unavailable in the test environment, so it was stubbed only to import the library):
Result: 2 passed.