-
Notifications
You must be signed in to change notification settings - Fork 20
ImgMath blockread and offset #88
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
Conversation
180ed00
to
35f0faa
Compare
…Function being the first place where the view methods are declared and defined.
some amount from it. It's like an ImgSource but for RandomAccessible instead of for RandomAccessibleInterval. Only works with RandomAccess and not with Cursor, so Compute had to adapt.
KDTreeSource reads from a KDtree<RealType> BlockRead reads sums from an integral image. Generalized with interface SourceInterval so that ImgSource and KDTreeSource both return an Interval. Util: new functions findFirstInterval, and wrap can now also handle a RandomAccessible as a source.
used for removing superfluous ops.
(pruned of null ops)
35f0faa
to
4215007
Compare
I merged #87 and rebased this PR over top the newest master. |
to be used as radius for all dimensions.
for consistency with e.g. ArrayImgs.floats
retrieve its RandomAccessibleInterval source if no other is found. New function Util.dimensions is even more flexible in trying to find an EuclideanSpace.
…mputeType that can differ from the outputType. When both are the same, avoids additional performance costs by removing the need for a Converter.
752ec7e
to
066eb2e
Compare
066eb2e
to
f45eb5c
Compare
with implementations as plain type operators and also an optimization as boolean operators for use in e.g. an if statement.
Ready to merge as far as I can tell. |
Hi @ctrueden, are there any adjustments to the POM that should be made prior to merging, or can it be merged as is? Thanks! |
It doesn't look like you relied on any unreleased functionality from SNAPSHOTs, so probably it's all good. Tangentially: you can use this script to check that all commits on the branch compile with passing tests. Here is the result on my system with the current branch: check-branch.sh
So e93de98 breaks the build, and e8c7e43 (five commits later) fixes it. This will hamper use of Aside from that, one big problem I notice is that there are no unit tests for these new functions. It's really important to add some before merging. I'm not a primary maintainer of this component—that's @tpietzsch and @axtimwalde and @StephanPreibisch. One of them should review and merge this work, not you or me. |
Thanks @ctrueden, regarding the 5 commits that fail, I'm happy to merge all those 5 commits into a single one, but it seems that would only make matters worse. Each change is small in any case, which is why I committed them separately for documentation. Regarding tests: I understand automatic testing is desirable. I do all my testing in scripts, as this library is meant to be used from scripts, to enable bypassing performance limitations of low-level code in some scripting languages. If writing tests from java as JUnit tests in this very same repository is a requirement, I'll try to find time for that eventually. They weren't there before for this library. |
3f0ce6f
to
08dd4a5
Compare
rather than a RandomAccessibleInterval.
I have now added tests for all new classes as requested. |
Given that the creation of ImgMath sublibrary was approved and this pull request merely adds additional classes in the same tune, without changing anything else outside of the net.imglib2.algorithm.math package, I will go ahead and merge this pull request in a few days unless objections are raised. |
Additional features for ImgMath, including:
RandomAccessible
as input, which enables e.g. offset reading, to create e.g. convolution filters or integral images (by reading from the prior pixel and writing onto the same image as the source).IntegralImg
or any other form of integral image (also known as sum area tables) expressed as aRandomAccessible
.KDTree
plus a radius as input.Math.log
andMath.exp
, not based onNumericType
math because this type lacks these functions.