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

add example SDR for binary and fuzzy images #1561

Merged
merged 3 commits into from Jun 29, 2018

Conversation

Projects
None yet
5 participants
@Borda
Copy link
Contributor

Borda commented Jun 11, 2018

Writing a simple example of registering binary and fuzzy images together with addressing tuning some particular parameters. see #1541

@pep8speaks

This comment has been minimized.

Copy link

pep8speaks commented Jun 11, 2018

Hello @Borda, Thank you for updating !

Cheers ! There are no PEP8 issues in this Pull Request. 🍻

Comment last updated on June 25, 2018 at 10:48 Hours UTC

@Borda Borda referenced this pull request Jun 11, 2018

Closed

demon registration, unstable? #1541

6 of 6 tasks complete
@arokem
Copy link
Member

arokem left a comment

Thanks for this example! I had a few suggestions for changes in wording.

Diffeomorphic Registration with binary and fuzzy images
=========================================================
This example for registering binary and fuzzy images together.
This may be seen as aligning sensed (fuzzy) image to the

This comment has been minimized.

@arokem

arokem Jun 12, 2018

Member

Typo: "to the and"

How about rewriting these two sentences as.

"This example demonstrates registration of a binary and a fuzzy image. This could be seen as aligning a fuzzy (sensed) image to a binary (e.g., template) image."

from dipy.viz import regtools

"""
Let's generate sample template image as combination of three ellipses.

This comment has been minimized.

@arokem

arokem Jun 12, 2018

Member

=> "Let's generate a sample template image as the combination of three ellipses"


"""
Let's generate sample template image as combination of three ellipses.
The fuzzy (sensed) is a smooth version of the reference image.

This comment has been minimized.

@arokem

arokem Jun 12, 2018

Member

=> "We will generate the fuzzy (sensed) version of the image by smoothing the reference image"

img_in = filters.gaussian(img_ref, sigma=3)

"""
Let's write down a short visualisation function.

This comment has been minimized.

@arokem

arokem Jun 12, 2018

Member

=> "Let's define a small visualization function"

"""

"""
Let's use the use the general Registration function with some naive parameters,

This comment has been minimized.

@arokem

arokem Jun 12, 2018

Member

The words "use the" are duplicated.

"""
Unfortunately, we did not realised that we are still using multi scale approach
which makes `step_length` in the upper level multiplicatively larger.
Let's experiment with `step_length` and set as quite small.

This comment has been minimized.

@arokem

arokem Jun 12, 2018

Member

How about changing this sentence to a question? Like so:

"What happens if we set step_length to a rather small value?"

sdr.step_length = 0.1

"""
Perform the registration and see output.

This comment has been minimized.

@arokem

arokem Jun 12, 2018

Member

=> "Perform the registration and examine the output."

.. figure:: map-2.png
:align: center
Registration results for decrease learning step.

This comment has been minimized.

@arokem

arokem Jun 12, 2018

Member

=> "Registration results for decreased step size."

"""

"""
Another alternative for such scenario is using just single scale level.

This comment has been minimized.

@arokem

arokem Jun 12, 2018

Member

=> "An alternative scenario is to use just a single scale level"


"""
Another alternative for such scenario is using just single scale level.
Although the warped image may look fine the estimated deformations is quite wild.

This comment has been minimized.

@arokem

arokem Jun 12, 2018

Member

=> "Even though the warped image may look fine, the estimated deformations show that it is off the mark."

@arokem

This comment has been minimized.

Copy link
Member

arokem commented Jun 12, 2018

Also, once you've made these changes, please take a look at the results from PEP8 speaks. In particular, it looks as though your editor is not set to remove trailing white-spaces, so please do remove these by hand.

@codecov-io

This comment has been minimized.

Copy link

codecov-io commented Jun 12, 2018

Codecov Report

Merging #1561 into master will decrease coverage by <.01%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1561      +/-   ##
==========================================
- Coverage   87.42%   87.42%   -0.01%     
==========================================
  Files         246      246              
  Lines       31458    31458              
  Branches     3425     3425              
==========================================
- Hits        27502    27501       -1     
  Misses       3144     3144              
- Partials      812      813       +1
Impacted Files Coverage Δ
dipy/core/graph.py 73.8% <0%> (-1.2%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 0bfa3ac...8e67db3. Read the comment docs.

@Borda

This comment has been minimized.

Copy link
Contributor

Borda commented Jun 13, 2018

@arokem thanks for help with the rewording :)

@arokem
Copy link
Member

arokem left a comment

Still a couple more comments left unaddressed. Once those are in, I am +1 for merging this. Could someone else also take a look, please?

opt_tol=1.e-3)

"""
Perform the registration in equal images.

This comment has been minimized.

@arokem

arokem Jun 15, 2018

Member

Still needs rewording here.

"""

"""
Perform the registration on binary and fuzzy images.

This comment has been minimized.

@arokem

arokem Jun 15, 2018

Member

And here

@Borda

This comment has been minimized.

Copy link
Contributor

Borda commented Jun 15, 2018

ok, and what do you recommend for these tows?

@arokem

This comment has been minimized.

Copy link
Member

arokem commented Jun 15, 2018

@Borda Borda force-pushed the Borda:example-sdr branch from 557a17d to 8fc44dd Jun 15, 2018

@Borda

This comment has been minimized.

Copy link
Contributor

Borda commented Jun 15, 2018

btw, Travis: The log length has exceeded the limit of 4 MB (this usually means that the test suite is raising the same exception over and over).

@arokem

This comment has been minimized.

Copy link
Member

arokem commented Jun 16, 2018

Yeah - that's unrelated. It's due to a new version of numpy (?). Also reported on #1565

@arokem

This comment has been minimized.

Copy link
Member

arokem commented Jun 16, 2018

This PR is 👍 for the merge from my side.

@Borda

This comment has been minimized.

Copy link
Contributor

Borda commented Jun 25, 2018

@arokem or anyone else, any other comment? :)

@skoudoro

This comment has been minimized.

Copy link
Member

skoudoro commented Jun 25, 2018

Nice Tutorial, Thanks @Borda!

Can you add your example in valid_examples and in examples index list please.

Thank you

@Borda

This comment has been minimized.

Copy link
Contributor

Borda commented Jun 27, 2018

@skoudoro done :)

@arokem

This comment has been minimized.

Copy link
Member

arokem commented Jun 29, 2018

Since no one had any more comments here, I am going to go ahead and merge this.

@arokem arokem merged commit 9015dca into nipy:master Jun 29, 2018

1 check failed

continuous-integration/travis-ci/pr The Travis CI build failed
Details

ShreyasFadnavis pushed a commit to ShreyasFadnavis/dipy that referenced this pull request Sep 20, 2018

Merge pull request nipy#1561 from Borda/example-sdr
add example SDR for binary and fuzzy images
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment