Skip to content

[WIP] Refactor in-place dispatch route – improve developer docs - #630

Draft
kellertuer wants to merge 110 commits into
masterfrom
kellertuer/refactor-eval
Draft

[WIP] Refactor in-place dispatch route – improve developer docs#630
kellertuer wants to merge 110 commits into
masterfrom
kellertuer/refactor-eval

Conversation

@kellertuer

@kellertuer kellertuer commented Jul 26, 2026

Copy link
Copy Markdown
Member

This PR resolves #610. Since it is a larger refactor, I wanted to open this PR a bit earlier so it is visible and people are aware, the code base is changing quite a bit.

Goal of this PR / Refactor

  • objectives will loose their EvaluationType - it was not reflected in any of the fields per se and a bit clumsy anyways (and maybe just a very early wrong design decision back then)
  • functions inside the objective will “earn” this instead. They will be the former in-place by default
  • a wrapper for the former allocating case allows to more flexibly have allocating functions in the objective
  • the former plan/ folder is split into
    • a base/ folder defining and describing interfaces
    • a commons/ folder collecting implementations several solvers have in common, like stopping criteria
    • some parts from the plan/ also move to their respective solvers, if there is no incentive to have these in the commons area

While parameters of several structs change, especially all objectives “loose” their first parameter, the hope is that in the end all constructors stay the same and hence this PR is non-breaking.

Roadmap

  • refactor all objectives by removing the evaluation type, refactor all corresponding accessor functions, temporarily remove all evaluation = keywords thereon
  • refactor all functions from the plan/ to base/ and commons/ – as a structured way to work through all points from the previous item
  • Refactor the docs into a developer guide and a commons area (instead of the plan) once Manopt complies again
  • reintroduce the evalution = to the objectives such that they wrap their function components accordingly. This would mean “wrapping” all functions to (internally) be allocating.
  • (maybe) additionally find a way to have a more fine granular interface to set single functions to be allocating. Maybe also just the new AllocatingManifoldFunction server well enough so wrap single allocating functions
  • carefully check that all tests work again, hopefully non-breaking.

TLDR

The information “in-place or allocating” gets “more modular” within the objective, code complexity is reduced – and we get developer docs as a side aspect.

…ameter from the AbstractManifoldObjective – introduce a wrapper AllocatingManifoldFunction instead.
@kellertuer kellertuer changed the title [WIP] Refactor in-place dispatch route – improve developer docs [WIP, early stage] Refactor in-place dispatch route – improve developer docs Jul 26, 2026
@kellertuer kellertuer changed the title [WIP, early stage] Refactor in-place dispatch route – improve developer docs [WIP] Refactor in-place dispatch route – improve developer docs Aug 3, 2026
@kellertuer kellertuer linked an issue Aug 3, 2026 that may be closed by this pull request
@codecov

codecov Bot commented Aug 5, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 98.33575% with 46 lines in your changes missing coverage. Please review.
✅ Project coverage is 99.40%. Comparing base (5e362ee) to head (ffdb0a2).

Files with missing lines Patch % Lines
src/commons/functions.jl 86.56% 18 Missing ⚠️
src/commons/vectorial_functions.jl 94.40% 9 Missing ⚠️
src/base/repl.jl 70.83% 7 Missing ⚠️
src/base/objective/first_order.jl 85.36% 6 Missing ⚠️
src/commons/sub_objectives.jl 98.03% 5 Missing ⚠️
src/base/objective/sub_objective.jl 94.73% 1 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff             @@
##            master     #630      +/-   ##
===========================================
- Coverage   100.00%   99.40%   -0.60%     
===========================================
  Files          102       90      -12     
  Lines        12957    12521     -436     
===========================================
- Hits         12957    12446     -511     
- Misses           0       75      +75     

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

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

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.

hasconverged(::StopWhenAll) reports false for nested criteria Work towards “in-place first” internally

1 participant