Skip to content
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

Replaced gpu<->cpu copies with host_callback in minimize. #253

Merged
merged 20 commits into from
Sep 14, 2022

Conversation

FernandoDavis
Copy link
Contributor

@FernandoDavis FernandoDavis commented Mar 16, 2022

Added the host_callback functionality using a lambda expression for the minimizer function.

  • Return value of minimizer function is a non Jax-type object. To fix incompatibility with hcb, I only extracted the x value from the response object.
  • This meant a change in tests was needed from out.x to just out.
  • Did not find a suitable workaround to let me keep the response object while also allowing the hcb to work. Missing data from this new format is a success boolean to check if minimize worked, a message that explains if it did not, and some other attributes listed: fun, jax, hess, etc.

Closes #204

@FernandoDavis
Copy link
Contributor Author

I'll check why the pytest is failing now in a few.

@bwohlberg bwohlberg added the improvement Improvement of existing code, including addressing of omissions or inconsistencies label Mar 23, 2022
@codecov
Copy link

codecov bot commented Mar 27, 2022

Codecov Report

Merging #253 (d6e0d30) into main (353a51d) will increase coverage by 0.02%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##             main     #253      +/-   ##
==========================================
+ Coverage   93.93%   93.95%   +0.02%     
==========================================
  Files          58       58              
  Lines        3495     3493       -2     
==========================================
- Hits         3283     3282       -1     
+ Misses        212      211       -1     
Flag Coverage Δ
unittests 93.95% <100.00%> (+0.02%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
scico/solver.py 99.25% <100.00%> (+0.72%) ⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@bwohlberg
Copy link
Collaborator

Ready for review?

@FernandoDavis
Copy link
Contributor Author

Yes! I'll be available if anything comes up.

scico/solver.py Outdated Show resolved Hide resolved
scico/solver.py Outdated Show resolved Hide resolved
scico/solver.py Outdated Show resolved Hide resolved
scico/solver.py Outdated Show resolved Hide resolved
scico/solver.py Outdated Show resolved Hide resolved
scico/solver.py Outdated Show resolved Hide resolved
@bwohlberg bwohlberg added this to the Release 0.0.3 milestone Sep 14, 2022
@FernandoDavis FernandoDavis merged commit 257f013 into main Sep 14, 2022
@FernandoDavis FernandoDavis deleted the FernandoDavis/Manual-gpu-copy-to-HCB branch September 14, 2022 15:19
@FernandoDavis
Copy link
Contributor Author

FernandoDavis commented Oct 11, 2022 via email

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
improvement Improvement of existing code, including addressing of omissions or inconsistencies
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Replace manual GPU<->host copies in scico.solver with host callback
3 participants