Skip to content

Add an optional callback to Lanczos#55

Merged
RaulPPelaez merged 8 commits intomainfrom
callbacks
Aug 14, 2025
Merged

Add an optional callback to Lanczos#55
RaulPPelaez merged 8 commits intomainfrom
callbacks

Conversation

@RaulPPelaez
Copy link
Contributor

This PR adds a new optional parameter to initialize, a callable that takes an integer and a float.
If passed and lanczos is used to compute the noise the callback will be invoked with the current iteration number and the current error after each iteration.

@github-actions
Copy link

github-actions bot commented Jul 22, 2025

Linter reported no issues

All Python files are correctly formatted with Black.

@github-actions
Copy link

github-actions bot commented Jul 22, 2025

Linter reported no issues

All C/C++ files are correctly formatted with clang-format.

@rykerfish
Copy link
Contributor

Seems like that did it- for some reason, my thrust libraries are worse than yours so the implicit cpu->gpu copy failed when the CUDA execution policy is specified. Do you think we should add an issue to convert the rest of lanczos to be purely GPU, if that's a feasible task? If so, I'd give it a go sometime in the future.

Copy link
Contributor

@rykerfish rykerfish left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good! My small change fixed the crash I was having so it seems good to go.

@RaulPPelaez
Copy link
Contributor Author

AFAICT, all the operations I left in the CPU have to be in the CPU. Some decisions must be performed CPU-side (like convergence checking).
I am sure that more performance can be squeezed out of it with some careful profiling, though. If you want to take a go at it, that would be great, for sure.

@RaulPPelaez RaulPPelaez merged commit a18c800 into main Aug 14, 2025
5 checks passed
@rykerfish rykerfish deleted the callbacks branch August 14, 2025 19:17
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