Add distance gradient

[kellertuer]

I might also add the geodesic derivatives here, but I also have to check, there are quite some warning with the docs it seems?

[codecov[bot]]

Codecov Report

Merging #15 (f7fee79) into main (6ddb681) will increase coverage by 0.82%. The diff coverage is 100.00%.

:exclamation: Current head f7fee79 differs from pull request most recent head c24ac70. Consider uploading reports for the commit c24ac70 to get more accurate results

@@            Coverage Diff             @@
##             main      #15      +/-   ##
==========================================
+ Coverage   91.69%   92.51%   +0.82%     
==========================================
  Files          13       15       +2     
  Lines         349      374      +25     
==========================================
+ Hits          320      346      +26     
+ Misses         29       28       -1     

Impacted Files	Coverage Δ
src/ManifoldDiff.jl	96.00% <ø> (+4.00%)	:arrow_up:
src/diagonalizing_projectors.jl	100.00% <ø> (ø)
src/riemannian_diff.jl	96.36% <ø> (ø)
src/derivatives.jl	100.00% <100.00%> (ø)
src/differentials.jl	100.00% <100.00%> (ø)
src/gradients.jl	100.00% <100.00%> (ø)

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

[mateuszbaran]

Yes, IIRC I haven't solved all docs warning, sorry.

[kellertuer]

No problem, I will probably fix them along the way as well then (already fixed about half of them). Just wanted to make sure, that it was not me introducing them ;)

[mateuszbaran]

Cool, thanks :+1:

[kellertuer]

I have (besides a strange duplicate docs error that I still have) a naming issue that I noticed.

For the differentials we put the differential_ first but for inverse_retract_diff_argument_fd_approx we did not. Even more the diff is shortened. Should we change that?

[kellertuer]

The differentials.jl reports duplicate entries for the adjoint differentials. I have no clue why, since they are not included and not related to that in any ways. Very strange. But the rest of the docs is refined.

[kellertuer]

This now resolves #13 and resolves #14.

[mateuszbaran]

For the differentials we put the differential_ first but for inverse_retract_diff_argument_fd_approx we did not. Even more the diff is shortened. Should we change that?

Yes, that's somewhat inconsistent. We also have for example translate_diff in Manifolds.jl. I'm don't have a good idea what to do about it.

The differentials.jl reports duplicate entries for the adjoint differentials. I have no clue why, since they are not included and not related to that in any ways. Very strange. But the rest of the docs is refined.

That's quite weird, yes.

[kellertuer]

I think at least here we should aim for a consistent naming then.

I propose following: We use one keyword as first keyword before the function, since that is what we did until now and the math operator is also usually upfront. For now we just have to adapt the one I mentioned and the just added gradient functions.

For the docs – the result looks good, just the warnings are strange and I do not see where those duplicates come from.

edit: Ah since the math notation also puts the ' behind the function, maybe also the last word is fine as long as it is consistent (e.g. all derivatives have the same name-format).

[kellertuer]

Just as a small Update here: ManoptExamples is in principle done in setup and just waiting for this PR to be finished (then it needs a small update to use the gradient from here).

[mateuszbaran]

OK, keyword before function name sounds fine.

ManoptExamples is in principle done in setup and just waiting for this PR to be finished (then it needs a small update to use the gradient from here).

Cool :+1:

[kellertuer]

So can we rename

inverse_retract_diff_argument_fd_approx

to

differential_inverse_retract_argument_fd_approx

?

[kellertuer]

I followed my ida and this is ready to review.

[mateuszbaran]

Yes, I think that renaming is fine.

[kellertuer]

Just found another small typo when experimenting over at Maniopt.jl – but if we could merge this and release 0.2.2 I could register ManoptExamples.jl (yay!).

[kellertuer]

Sure due to the renaming that might be the safe way to go.