Skip to content

Conversation

@andres-fr
Copy link
Collaborator

The main scope of this PR is to address the following issue:

Scope and performance of diagonal approximations #198

Proposed changes in this PR

  • Removed stochastic diagonal and trace estimators from the curvlinops codebase
  • Removed subsequent tests and ensured API docs remain consistent
  • Updated trace/diagonal example from the gallery to use skerch functionality instead
  • Added __rmatmul__ to the examples tensor linop to allow for skerch interoperability (shouldn't harm other linops from the core lib)
  • Added skerch to the test dependencies

As a result, trace and diagonal estimators are now totally out of the scope of curvlinops, but the gallery example shows that it integrates nicely with skerch, and the errors even got lower!


Additionally, this PR also proposes the following changes, which are not related to the issue above, and I needed in order to properly lint/test the repo and compile the docs:

  • Raised python support from 3.9 to 3.10: Python 3.9 is deprecated and caused dependency issues
  • Also fixed pytest-optional-tests version to 0.1.1, newer versions cause make test to crash
  • Fixed a bunch of pydoctest errors, and excluded docs from pydoctest (many errors were IMO unfixable and wouldn't make sense to cover, plus pydoctest doesn't allow to exclude/include subdirectories as per e.g. this and this issues.
  • Fixed CUDA code, which was broken in most examples (e.g. plotting CUDA tensors, which doesn't work)

As a result, all make commands now work from a fresh install on my end (Ubuntu system with CUDA GPU).


I managed to do all these changes fast enough to be on top of main HEAD, so although some of them are fundamental and out of scope, hopefully we can land on something that is good for the project. Happy to hear your feedback and adapt!

Also, happy holidays :)
Andres

@andres-fr andres-fr requested a review from f-dangel December 20, 2025 20:49
@f-dangel
Copy link
Owner

f-dangel commented Dec 26, 2025

Hey, I quickly skimmed through the changes.

Could we split up this PR into the following?

  1. Deprecate Python 3.9
  2. General CI fixes and docstring improvements
  3. Remove diag/trace/frobenius estimators

@andres-fr
Copy link
Collaborator Author

Sure, how do we do this? Should I propose 3 individual PRs?

@f-dangel
Copy link
Owner

Yes 👍

@andres-fr andres-fr mentioned this pull request Jan 6, 2026
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