Differences #44
Differences #44
Conversation
tibuch
commented
Jun 22, 2017
tibuch
commented
- Remove the nice but slow implementation of central difference.
- Add forward difference
- Add backward difference
|
I would rather leave the nice and slow version for educational purposes. Just comment it out, but leave it in. |
|
Removed the remove commit. |
|
Please add test cases for small 2D and 3D images. |
hanslovsky
left a comment
hanslovsky
left a comment
There was a problem hiding this comment.
Minor additions to JavaDoc
| } | ||
|
|
||
| /** | ||
| * Compute the backward difference of source in a particular dimension. |
There was a problem hiding this comment.
Please at some JavaDoc math that specifies exactly what is being computed, like
https://en.wikipedia.org/wiki/Finite_difference#Forward,_backward,_and_central_differences
There was a problem hiding this comment.
Using the LoopBuilder implementation by @maarzt makes the code much more readable. If necessary I can add more java doc.
There was a problem hiding this comment.
Having the math in the doc would be very helpful, either in code
d_f( x ) = f( x + h ) - f( x )
or tex
(examples for forward difference).
There was a problem hiding this comment.
The function f in the equation d_f( x ) = f( x + h ) - f( x ) has an one dimensional domain. If we want to describe what gradientForwardDifference actually computes on 2d, 3d or Nd images, we would need a more complicated equation.
I think the resulting math will be rather confusing. @hanslovsky Please proof me wrong!
| } | ||
|
|
||
| /** | ||
| * Compute the forward difference of source in a particular dimension. |
There was a problem hiding this comment.
Please at some JavaDoc math that specifies exactly what is being computed, like
https://en.wikipedia.org/wiki/Finite_difference#Forward,_backward,_and_central_differences
There was a problem hiding this comment.
Having the math in the doc would be very helpful, either in code
d_f( x ) = f( x + h ) - f( x )
or tex
(examples for forward difference).
| <groupId>net.imglib2</groupId> | ||
| <artifactId>pom-imglib2</artifactId> | ||
| <version>10.0.1</version> | ||
| <version>10.0.2</version> |
There was a problem hiding this comment.
Why do we need the bump here? Is it for the LoopBuilder?
There was a problem hiding this comment.
Yes, it's because this makes LoopBuilder available.
I changed commit message, to include that information.
| @@ -0,0 +1,94 @@ | |||
| package net.imglib2.algorithm.gradient; | |||
|
Added some minor comments but then it looks good to merge for me. |
This makes the class LoopBuilder available.
Addresses comments in imglib#44 Also adds formulae for central difference
|
Looks good to me now. Will leave open for a bit for more comments/concerns and then merge eventually. |
4 participants
