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

Resolve some components of #228 #340

Merged
merged 32 commits into from
Sep 14, 2022
Merged

Resolve some components of #228 #340

merged 32 commits into from
Sep 14, 2022

Conversation

bwohlberg
Copy link
Collaborator

Resolve some components (as indicated in issue) of #228.

@bwohlberg bwohlberg added documentation Improvements or additions to documentation examples Pertaining to SCICO examples labels Sep 13, 2022
@codecov
Copy link

codecov bot commented Sep 13, 2022

Codecov Report

Merging #340 (69a2c1a) into main (257f013) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##             main     #340   +/-   ##
=======================================
  Coverage   93.95%   93.95%           
=======================================
  Files          58       58           
  Lines        3493     3493           
=======================================
  Hits         3282     3282           
  Misses        211      211           
Flag Coverage Δ
unittests 93.95% <ø> (ø)

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

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



Sparse Coding
^^^^^^^^^^^^^

`sparsecode_admm.py <sparsecode_admm.py>`_
Non-negative Basis Pursuit DeNoising (ADMM)
Non-Negative Basis Pursuit DeNoising (ADMM)
Copy link
Contributor

Choose a reason for hiding this comment

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

Did we find a consensus on how we handle capitalization?
I find it odd to read COde or DeNoising
@Michael-T-McCann

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This is non-standard capitalization, but it's the one originally used to justify the BPDN acronym. I'm open to alternatives if anyone finds this version truly offensive.

framework :cite:`venkatakrishnan-2013-plugandplay2`, using BM3D
This example demonstrates solution of a tomographic reconstruction
problem using the Plug-and-Play Priors framework
:cite:`venkatakrishnan-2013-plugandplay2`, using BM3D
:cite:`dabov-2008-image` as a denoiser and SVMBIR :cite:`svmbir-2020` for
tomographic projection.

Copy link
Contributor

Choose a reason for hiding this comment

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

change version to example here as well?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Some rephrasing in both versions.

@@ -5,13 +5,12 @@
# with the package.

"""
CT Reconstruction (ADMM Plug-and-Play Priors w/ BM3D, SVMBIR+Prox)
==================================================================
PPP (with BM3D) CT Reconstruction (ADMM with SVMBIR Fast Prox)
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe call it Fast SVMBIR Prox?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Done.

Copy link
Contributor

@tbalke tbalke left a comment

Choose a reason for hiding this comment

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

Approve upon addressing a few minor comments.

@bwohlberg bwohlberg added this to the Release 0.0.3 milestone Sep 14, 2022
@bwohlberg bwohlberg merged commit b09c26b into main Sep 14, 2022
@bwohlberg bwohlberg deleted the brendt/issue#228 branch September 14, 2022 22:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation examples Pertaining to SCICO examples
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants