Closed kellertuer closed 3 weeks ago
Once you merge this PR into your default branch, you're all set! Codecov will compare coverage reports and display results in all future pull requests.
Thanks for integrating Codecov - We've got you covered :open_umbrella:
@mateuszbaran I struggle a bit with the next const-type-thing. This time LieAlgebra as a special type of Fiber.
using LieGroups
G = TranslationGroup(3)
LieAlgebra(G)
yields
TypeError: in Fiber, in G, expected G<:(LieGroup{ℝ, O, M} where {O<:AbstractGroupOperation, M<:ManifoldsBase.AbstractManifold{ℝ}}), got a value of type TranslationGroup{ℝ, ManifoldsBase.TypeParameter{Tuple{3}}}
Stacktrace:
[1] (ManifoldsBase.Fiber{𝔽, ManifoldsBase.TangentSpaceType, G, I} where {𝔽, G<:(LieGroup{𝔽, O, M} where {O<:AbstractGroupOperation, M<:ManifoldsBase.AbstractManifold{𝔽}}), I<:Identity})(G::TranslationGroup{ℝ, ManifoldsBase.TypeParameter{Tuple{3}}})
@ LieGroups ~/Repositories/Julia/LieGroups.jl/src/interface.jl:91
[2] top-level scope
@ REPL[4]:1
I am super buzzed by “expected Lie Group got TranslationGroup”, because TranslationGroup is a LieGroup (const)
besides that this PR only misses to check which other functions already to take over (from here). and maybe work a bit on the test suite further.
Besides fighting parameters in const (constantly it seems) – the interface seems usable I think, so we could start transferring more LieGroups afterwards.
It appears you just forgot one typeof
:slightly_smiling_face:
Oh noes! I was too busy checking the third parameter over and over again, that I did not spot that one – and probably got confused that the variable has a capital letter. Thanks!
I am also fond of getting the test suite better from the start here – slowly doing that along with the first group. If that works well I will adopt the same scheme over in Manifolds.jl
You're welcome.
I'm still curious how this testing approach works out but I guess I'll wait until it's a bit more complete.
If you have comments / questions feel free to post them, the general idea is:
With a bit of given data of (up to) 3 points, 3 vectors
properties
that list which functions or more general which things can be testedexpectations
for whenever we have to be able to know a resultFor now both properties
and expectations
are set manually, but I started already with an idea in the Manifolds.jl branch to have an “auto-discover” mode for them.
This should make testing concrete other groups later a bit faster.
I also tried to encapsulate all this in a small test package. so that this can easily be loaded and avoid the need for the “if not yet loaded include now” approach we have in a few packages for generic tools like a DummyLieGroup
for example.
The interface/fallbacks in general have to be done as a separate test anyways.
- provide
expectations
for whenever we have to be able to know a result
Could you add specific expectations to tested functions in the TranslationGroup
test suite? For example that exp
performs addition and log
does subtraction?
For now both
properties
andexpectations
are set manually, but I started already with an idea in the Manifolds.jl branch to have an “auto-discover” mode for them.
I think auto-discovery shouldn't be that important because when you develop some code, you know what should work. So it's not much work to add it manually. Maybe a little bit of semi-automatic discovery, such as "if you declared X and Y as implemented, Z should also work", would be useful though.
- provide
expectations
for whenever we have to be able to know a resultCould you add specific expectations to tested functions in the
TranslationGroup
test suite? For example thatexp
performs addition andlog
does subtraction?
Uff. No clue, that is maybe a bit too high-level.
For now exp
and log
are mainly checked to produce valid points and vectors, respectively and if both are available, that they are inverses of each others for the given points.
The expectations I have in mind is mainly numbers and strings – main example for now the string repr(G)
returns.
You idea sounds much more like a whole AI to implement to interprete results. That is not what expectation means – much simpler, just in the sense of expected values (for certain things).
For now both
properties
andexpectations
are set manually, but I started already with an idea in the Manifolds.jl branch to have an “auto-discover” mode for them.I think auto-discovery shouldn't be that important because when you develop some code, you know what should work. So it's not much work to add it manually. Maybe a little bit of semi-automatic discovery, such as "if you declared X and Y as implemented, Z should also work", would be useful though.
Sure auto.discover is something not really urgent and also a bit dangerous, since you would irregularly run it and it would save both properties and expectations in a .jld2
file. The danger is that it might also save erroneous expectatios then (in the sense of wrong values/strings). I would also not consider this too high priority, it was just something the manifolds-test suite branch experimented with a bit. For now we should maybe not follow this here.
Uff. No clue, that is maybe a bit too high-level. For now
exp
andlog
are mainly checked to produce valid points and vectors, respectively and if both are available, that they are inverses of each others for the given points.The expectations I have in mind is mainly numbers and strings – main example for now the string
repr(G)
returns. You idea sounds much more like a whole AI to implement to interprete results. That is not what expectation means – much simpler, just in the sense of expected values (for certain things).
That's fine but then this test suite is not sufficiently exhaustive to replace other tests. And I definitely don't mean anything related to AI, only letting the user provide reference results for specific operations such as exp and log. Maybe it's obvious but I wouldn't like to have an expectation that when test_LieGroup(G, properties, expectations)
passes, the group is definitely correctly implemented. Similarly test_manifold
only checks some general properties and is not exhaustive.
The suite is meant to provide a framework for common, usual tests. Sure that might not be exhaustive. I would even be open, that none of our tests are probably exhaustive (in favour of a finite time).
But the point that the suite is to provide tests for most common properties to check is probably a good thing to add to their docs. Though I think we might be able to cover as much as we usually test in Manifolds.jl for most manifolds (and not only with the test_manifold function, but a bit beyond).
I think I at least see an end to this with only a few small test suite functions and a few lines of code implementation left.
Speaking about testing, I wrote a package containing sensible tests for Lie groups. You can see an example of usage here: https://github.com/olivierverdier/ManifoldGroupTesting.jl/blob/main/test/test_group.jl There are also tests for group actions.
Nice! Here for now I just wrote a few functions like that in an internal sub-package, though here the blocks in each functions are actually already test sets, so they usually test a few more things than just one thing – for now they are a bit more focussed on low-level correctness – that the code works, yours already seem to also check the next level, that more formulae are verified.
In theory everything is documented by now.
In practice I fought “duplicate docs” errors for the last hour, that I have never seen before coming from one and the same autodocs block. Well. Maybe this magically disappears tomorrow, or someone else sees how to fix this.
Besides that this is finished.
Hmpf. Who ever came up with this clever idea in @autodocs
Note that page matching is done using the end of the provided strings and so a.jl will be matched by any source file that ends in a.jl, i.e. src/a.jl or src/foo/a.jl.
never wrote two interfaces in one project.
Hi, I didn't subscribe to LieGroups.jl notifications after it was transferred and now I'm far behind so sorry I can't help out here. It's very nice to see that you made a lot of progress!
No worries, hat was quite a technical hustle we went through, but (yay!) we got the name that way. Feel free to comment anything you notice or open issues about groups you would like to see. We can also already have your group in mind you mentioned, just open an issue for that as well.
I started looking a bit into ambiguities, and in general it looks nice, just that for example
+(Manifolds.Identity{O}, LieGroups.Identity{O})
is a bit of a temporary and annoying one. I will put that one on Ignore since it will resolve automatically once we make the breaking change in Manifolds.
So we now further
Aqua.jl
tests:Mutating => false
in the properties, either to be a bit faster or to work on a manifold with non-mutating points/vectors.
This PR aims to test our interface ideas with a first concrete Group operation and a Lie group
AdditionGroupOperation
TranslationGroup
exp
log
diff_conjugate
div_left_compose
,diff_right_compose
diff_inv
lie_bracket
is_point
and for identitycheck_vector
(a bit nicer than before)is_vector
inv_left_compose
,adjoint
,inv_right_compose
Identity
gets involved