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

Introduce a common type for differentiated retractions as vector transport and two such transports for Stiefel #318

Merged
merged 21 commits into from
Dec 29, 2020

Conversation

kellertuer
Copy link
Member

@kellertuer kellertuer commented Dec 24, 2020

Here's my little Christmas project. 🎄
I read a little bit about two new differentiated retractions (they are actually available in ROPTLIB (https://github.com/whuang08/ROPTLIB), but not that well documented.

  • Testing
  • Check Documentation
  • Check whether large interims matrices can be avoided (currently for example d*d' should be 1000x100 for Stiefel(1000,2) which should be avoided by changing order of compuations.

@kellertuer kellertuer added the WIP Work in Progress (for a pull request) label Dec 24, 2020
@codecov
Copy link

codecov bot commented Dec 24, 2020

Codecov Report

Merging #318 (c9304b4) into master (a1b604f) will increase coverage by 0.01%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #318      +/-   ##
==========================================
+ Coverage   95.97%   95.99%   +0.01%     
==========================================
  Files          69       69              
  Lines        4646     4667      +21     
==========================================
+ Hits         4459     4480      +21     
  Misses        187      187              
Impacted Files Coverage Δ
src/Manifolds.jl 100.00% <ø> (ø)
src/manifolds/Stiefel.jl 97.79% <100.00%> (+0.33%) ⬆️
src/tests/tests_general.jl 95.86% <100.00%> (+0.04%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update a1b604f...4bd58af. Read the comment docs.

@kellertuer kellertuer added 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 and removed WIP Work in Progress (for a pull request) labels Dec 25, 2020
src/manifolds/Stiefel.jl Outdated Show resolved Hide resolved
src/manifolds/Stiefel.jl Outdated Show resolved Hide resolved
@mateuszbaran
Copy link
Member

The failure on Julia nightly is now caused by this: JuliaPy/PyCall.jl#873 .

@kellertuer
Copy link
Member Author

Thanks for investigating, I did not yet have the time to investigate (and was happy to get pyplot-stuff running in the first place, which took some time and tries).

@mateuszbaran
Copy link
Member

No problem, I think it's good to see why nightly builds are failing once in a while.

BTW, I just noticed that the description of scaled transport has some issues: https://juliamanifolds.github.io/Manifolds.jl/previews/PR318/interface.html#ManifoldsBase.ScaledVectorTransport .

@kellertuer
Copy link
Member Author

Thanks for noticing there is a space missing; I will push that fix to master and it will then get into the next version,

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.

OK, I don't have any other comments here 🙂 .

@kellertuer
Copy link
Member Author

Cool! I just bumped the version and will then merge in a few minutes.

@kellertuer kellertuer merged commit 2b3da3a into master Dec 29, 2020
@kellertuer kellertuer removed Ready-for-Review A label for pull requests that are feature-ready preview docs Add this label if you want to see a PR-preview of the documentation labels Dec 29, 2020
@kellertuer kellertuer deleted the kellertuer/differentiated-retraction branch January 5, 2021 21:07
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