-
Notifications
You must be signed in to change notification settings - Fork 10
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
Progress bar callback #78
base: master
Are you sure you want to change the base?
Conversation
Thanks, @atsanda for the PR! Can I quickly ask what the advantage of ProgressBar over the already supplied |
Hi @JakobAsslaender! There was a feature request in the issue I linked. This PR addresses that request. As far as I understand, |
It is possible to add add the callback only if ProgressMeter is loaded, however since we require a struct for it we need to do a few tricks. These can also be seen in LinearOperatorCollection. First we need to define an abstract ProgressMeterCallback, similar to for example this FFT op. This is so that we can export the ProgressMeterCallback in RegLS and workaround the fact that package extension are not designed to add new exports of structs and functions. Then we can add a new folder for our extesion, such as ext/ and in there add the ProgressMeterExtensionExt, similar to the FFT extension. This is a new module that loads both RegularizedLeastSquares and ProgressMeter. In this module we then define ProgressMeterCallbackImpl.jl, this is the actual struct. There we then have the same code as you previously had in this PR and you implement Lastly we need to change the Project.toml. There you need to add a weak dependency as well as an extension, similar to here. These changes would give us the functionality from before conditionally loaded with ProgressMeter |
Thank you @nHackel ! |
Project.toml
are worth reviewingRelated issue:
#71