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

Automatically Determine Manifold Features for Testing and Documentation #650

Closed
wants to merge 2 commits into from

Conversation

kellertuer
Copy link
Member

@kellertuer kellertuer commented Sep 12, 2023

In order to properly do #649, we need a way to determine which functions exist (at a certain time, cf. the discussion at #548) on a manifold. This revises an approach that I did in #548 already. This feature approach had two ideas (testing and documentation) I would like to finish before continuing on the testing.

The main part I am not yet sure about is how to provide the properties section – slightly whether that is useful in testing (embedding is, but the rest I am not sure), but mainly which parts one could add for the documentation here in a reasonable way.

@kellertuer kellertuer changed the title extract the old design of a features functions Automatically Determine Manifold Features for Testing and Documentation Sep 12, 2023
src/utils/features.jl Outdated Show resolved Hide resolved
src/utils/features.jl Outdated Show resolved Hide resolved
@codecov
Copy link

codecov bot commented Sep 12, 2023

Codecov Report

Merging #650 (f075781) into master (22dc01d) will decrease coverage by 1.15%.
The diff coverage is 0.00%.

@@            Coverage Diff             @@
##           master     #650      +/-   ##
==========================================
- Coverage   99.22%   98.07%   -1.15%     
==========================================
  Files         106      107       +1     
  Lines       10430    10552     +122     
==========================================
  Hits        10349    10349              
- Misses         81      203     +122     
Files Changed Coverage Δ
src/Manifolds.jl 86.66% <ø> (ø)
src/utils/features.jl 0.00% <0.00%> (ø)
src/utils/helpers.jl 100.00% <ø> (ø)

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@mateuszbaran
Copy link
Member

The main part I am not yet sure about is how to provide the properties section – slightly whether that is useful in testing (embedding is, but the rest I am not sure), but mainly which parts one could add for the documentation here in a reasonable way.

What other properties would you put there? I'd imagine you could for example consider is_flat a property but it raises an interesting problem: some manifolds are flat only for certain parameter values. Related issue: some functions may only be implemented for certain parameter values (see for example quaternionic unitary group or IIRC Kendall's shape spaces). Ideally we would do full tests only for one set of parameters and for other only test functions with meaningful differences. For documentation, we may want a table for each manifold that has different feature entries for different size/parameter ranges.

@kellertuer
Copy link
Member Author

My idea is to collect a few “features” that might be nice to know in a summary table (that lists functions, retractions, inverse retractions, vectors transports) for a manifold. For example whether it has an embedding or so. But really just things that are known generically.

@mateuszbaran
Copy link
Member

Hm, "things that are known generically" sounds very restrictive to me. I'd guess that for everything other than manifold_dimension and maybe rand I could give an example of manifold where the operation is only available for certain parameters. Quaternionic unitary matrices are a great example.

@kellertuer
Copy link
Member Author

manifold dimension and rand would not fit so much; as I said I am not yet 100% sure, my main idea was to collect traits.

@mateuszbaran
Copy link
Member

If you just want traits then there won't be much as IMO traits should be limited to what we need to dispatch on. Also,

    functions::Vector{F}
    retractions::Vector{AbstractRetractionMethod}
    inverse_retractions::Vector{AbstractInverseRetractionMethod}
    vector_transports::Vector{AbstractVectorTransportMethod}

doesn't look like collecting traits?

@kellertuer
Copy link
Member Author

I was only speaking about the properties part.

The ones you mention – that is the easy and solved part – I collect functions, retractions, inverse retractions an vector transports that are available on some M.

But for automatic testing features like – does it have an embedding and is it isometric would be cool to generically know. That is what I have not yet modelled completely here.

@mateuszbaran
Copy link
Member

The ones you mention – that is the easy and solved part – I collect functions, retractions, inverse retractions an vector transports that are available on some M.

I was actually commenting that it's only easy when you have fixed parameters for a manifold. For example a table for UnitaryMatrices(1, ℍ) looks different than for UnitaryMatrices(2, ℍ). Did you take it into account?

@kellertuer
Copy link
Member Author

For the tests that is intentional – for the docs and displaying a summary that might be something to still check.

@mateuszbaran
Copy link
Member

OK.

One more point, when re-running feature discovery it would be good to have something for comparing two saved feature sets to make sure the newly added feature is correctly discovered and no old features are marked as not present. Maybe just saving to a text-based format so that standard diff tools work or, if the format is binary, a function that reports changes.

@kellertuer
Copy link
Member Author

That might be details a bit more necessary on the other branch, but I did not plan to design a full text file export, but just save the jd2. One could still to a set-based difference report if something changes – especially if something is removed.

@kellertuer
Copy link
Member Author

Concerning the properties, I will aim to do a minimal approach here and keep that flexible for now, it depends a bit on what we can do with that in the tests in the end.

@kellertuer
Copy link
Member Author

kellertuer commented Sep 19, 2023

So for now the features produce something like

julia> Manifolds.ManifoldFeatures(Sphere(2))
ManifoldFeatures


Functions
  * copy
  * distance
  * embed
  * exp
  * geodesic
  * injectivity_radius
  * inverse_retract
  * is_point
  * is_vector
  * log
  * manifold_dimension
  * norm
  * parallel_transport_direction
  * parallel_transport_to
  * rand
  * representation_size
  * retract
  * riemann_tensor
  * shortest_geodesic
  * vector_transport_direction
  * vector_transport_to
  * zero_vector

Retractions
  * ExponentialRetraction
  * ProjectionRetraction

Inverse Retractions
  * LogarithmicInverseRetraction
  * ProjectionInverseRetraction

Vector transports
  * ParallelTransport
  * PoleLadderTransport
  * ProjectionTransport
  * ScaledVectorTransport
  * SchildsLadderTransport

And I think I would like to keep it at that for now. The features will be nice for knowing whether the manifold is embedded and in what sense (so one could spare an inner test for example); But I would pefer to merge this soon to continue on the other branch.

Still nt yet completly determined here (still to check how to best find that)

  • include the list above as a nice summary in the docs – might be problematic in cases where existence of functions / retractions depends on certain point/vectors and/or dimensions. Also to properly include this it might be necessary to not only do a markdown but an HTML export – one could put the list to the right of the docs page. Sadly I also have no clue how to link eventually downwards if the docs exist (not for inherited inner for example). So since this seems really a lot of unknown, let's just keep this features parts as is here and finish this
  • save/load/update these features, but that is also something I would like to work on in the test branch.
  • Should I add a test to this? Or should we exclude that file for now from testing?

@kellertuer
Copy link
Member Author

This seems to work reasonably well, but I will merge this into the other PR, since I can not do that well of a testing here;

@kellertuer kellertuer closed this Sep 20, 2023
@kellertuer kellertuer deleted the kellertuer/manifold_features branch May 4, 2024 17:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants