Skip to content

Update ManifoldDiff.jl compat to 0.4 and CI#23

Merged
mateuszbaran merged 17 commits intomainfrom
mbaran/update-compat-manifolddiff-0.4
Feb 11, 2025
Merged

Update ManifoldDiff.jl compat to 0.4 and CI#23
mateuszbaran merged 17 commits intomainfrom
mbaran/update-compat-manifolddiff-0.4

Conversation

@mateuszbaran
Copy link
Copy Markdown
Member

No description provided.

@mateuszbaran mateuszbaran added the Ready-for-Review A label for pull requests that are feature-ready label Dec 15, 2024
@codecov
Copy link
Copy Markdown

codecov Bot commented Dec 15, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 93.26%. Comparing base (61697cf) to head (6c76c5e).
Report is 1 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main      #23      +/-   ##
==========================================
+ Coverage   93.03%   93.26%   +0.22%     
==========================================
  Files          11       11              
  Lines        1191     1187       -4     
==========================================
- Hits         1108     1107       -1     
+ Misses         83       80       -3     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Comment thread examples/Bezier-curves.qmd
@mateuszbaran
Copy link
Copy Markdown
Member Author

@kellertuer I have no idea why building the tutorial gets stuck on CI. Could you try it on your computer? I still don't have a working quarto installation.

@kellertuer
Copy link
Copy Markdown
Member

Hm, clear cache here, and maybe also let the examples run on 1.11?

@mateuszbaran
Copy link
Copy Markdown
Member Author

There are no caches in this repository and the example should work fine on Julia 1.10.

@kellertuer
Copy link
Copy Markdown
Member

Ok, I can try, but probably only tomorrow, just came home and first need something to eat.

@kellertuer
Copy link
Copy Markdown
Member

Bumped all versions (Julia for documenter and quarto, quarto, version on the example manifest), my local version is on 15/21 in the nb that got stuck, a few seem to run a bit slow, yeah.

@kellertuer
Copy link
Copy Markdown
Member

Yeah maybe the subsolver calls take too long on cell 20/21, but I have no clue how to directly debug that for now.
This week seems to become the week everything fails if this continues.

@mateuszbaran
Copy link
Copy Markdown
Member Author

Maybe just setting a lower limit of iterations would be fine? At least as a temporary solution until you have time to investigate.

@kellertuer
Copy link
Copy Markdown
Member

Will check, the current parameters are the ones from the paper, so it would be a pity to change that, but well, maybe that is what it is and it turns out nearly nothing is ever reproducible.

@mateuszbaran
Copy link
Copy Markdown
Member Author

It might be a good idea to revive this PR a bit?

@kellertuer
Copy link
Copy Markdown
Member

Ah, the newest version should already run on 0.4 now I think?

@mateuszbaran
Copy link
Copy Markdown
Member Author

This PR also has a few other minor changes

@kellertuer
Copy link
Copy Markdown
Member

Ah, I see. Sure feel free to check that it works with Manopt 0.5.6 once that is registered, maybe even good since for now we. have this circular dependency between those two.

@mateuszbaran
Copy link
Copy Markdown
Member Author

I've fixed the Riemannian difference-of-convex example but the Riemannian mean example still uses some old Manopt internals, could you take a look @kellertuer ?

@kellertuer
Copy link
Copy Markdown
Member

Sure but maybe not tomorrow

@kellertuer
Copy link
Copy Markdown
Member

Luckily I have my own changelog in mind and document constructors, I am not sure why the CG constructor was still on 0.3 style (nearly no keywords); It now has keywords, then I just had to adapt Armijo to the new Factory idea and then it ran fine. I did also add a “Tech” section to print package versions.

Comment thread .github/dependabot.yml
Comment thread Project.toml Outdated
@mateuszbaran
Copy link
Copy Markdown
Member Author

OK, I think we can merge this now 🙂

@mateuszbaran mateuszbaran merged commit 86f4d5e into main Feb 11, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

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.

2 participants