Skip to content
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

A first Lie group! #8

Open
wants to merge 62 commits into
base: main
Choose a base branch
from
Open

A first Lie group! #8

wants to merge 62 commits into from

Conversation

kellertuer
Copy link
Member

@kellertuer kellertuer commented Oct 9, 2024

This PR aims to test our interface ideas with a first concrete Group operation and a Lie group

  • Introduce AdditionGroupOperation
  • Introduce TranslationGroup
  • transfer default for the addition group (and test snippets)
    • exp
    • log
    • diff_conjugate
    • div_left_compose, diff_right_compose
    • diff_inv
    • lie_bracket
    • document them thoroughly to help people getting started what the terminology is
  • transfer in general
    • is_point and for identity check_vector (a bit nicer than before)
    • is_vector
  • start a test suite
    • add tests for the translation group
    • add generic tests with a dummy Lie group and a dummy Lie algebra to test a few fallbacks.
    • tests for also the default given ones like inv_left_compose, adjoint, inv_right_compose
  • reorganise the interface to have a bit of a shorter page – maybe a structure with sub pages for
    • general interface functions
    • group operations interface
    • group actions interface
  • check for ambiguities once the Identity gets involved
  • setup codecov
  • setup vale (though not yet run on CI)
  • setup/prepare zenodo metadata.

@kellertuer kellertuer added the preview docs Add this label if you want to see a PR-preview of the documentation label Oct 9, 2024
Copy link

codecov bot commented Oct 11, 2024

Welcome to Codecov 🎉

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 ☂️

@kellertuer
Copy link
Member Author

kellertuer commented Oct 13, 2024

@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.

@mateuszbaran
Copy link
Member

It appears you just forgot one typeof 🙂

@kellertuer
Copy link
Member Author

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!

@kellertuer
Copy link
Member Author

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

@mateuszbaran
Copy link
Member

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.

@kellertuer
Copy link
Member Author

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

  • provide modular test functions for every single function of the interface – potentially with interactions to others like in exp/log
  • provide properties that list which functions or more general which things can be tested
  • provide expectations for whenever we have to be able to know a result

For 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.

src/interface.jl Outdated Show resolved Hide resolved
@kellertuer kellertuer added the Ready-for-Review A label for pull requests that are feature-ready label Oct 17, 2024
Duplicate docs from the same file includion? Never seen that before.
@kellertuer
Copy link
Member Author

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.

@kellertuer
Copy link
Member Author

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.

Copy link
Member

@mateuszbaran mateuszbaran left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A few comments. I haven't checked everything yet.

docs/src/groups/translation.md Outdated Show resolved Hide resolved
docs/src/interface/operations.md Outdated Show resolved Hide resolved
docs/src/notation.md Outdated Show resolved Hide resolved
docs/src/notation.md Outdated Show resolved Hide resolved
docs/src/references.md Outdated Show resolved Hide resolved
src/Lie_algebra/Lie_algebra_interface.jl Outdated Show resolved Hide resolved
src/Lie_algebra/Lie_algebra_interface.jl Outdated Show resolved Hide resolved
src/group_actions/group_operation_action.jl Outdated Show resolved Hide resolved
src/interface.jl Outdated Show resolved Hide resolved
src/interface.jl Outdated Show resolved Hide resolved
@Affie
Copy link

Affie commented Oct 21, 2024

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!

@kellertuer
Copy link
Member Author

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.

@kellertuer
Copy link
Member Author

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.

@kellertuer
Copy link
Member Author

So we now further

  • run Aqua.jl tests
  • the test suite can now run less tests (skip mutating variants) by setting :Mutating => false in the properties, either to be a bit faster or to work on a manifold with non-mutating points/vectors.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
preview docs Add this label if you want to see a PR-preview of the documentation Ready-for-Review A label for pull requests that are feature-ready
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants