imagej-ops
imagej-ops copied to clipboard
Add (P)SNR, RMSE, and MAE
Adds implementations for (peak) signal-to-noise ratio, root mean square error, and mean absolute error of two images (a reference and a test image).
Closes #521.
Thanks for taking some time to look at his @imagejan!
Would it make sense to make the out parameter optional, or to add Function signatures in addition to these Computer ones? Or is it wrong to provide a default output type like e.g. DoubleType?
Totally makes sense, especially since the type parameter for the output is obsolete anyway (that was a relic from when I was trying to do all computations directly on the RealType
objects).
From the op signature (e.g. in the Op Finder) it wasn't clear to me which of in1 and in2 is the reference and which the test image. Maybe we can rename them to reference and test?
This would be amazing, though I fear that the information used in the Op Finder is extracted from the inherited instance variables in1
and in2
. I did at least change the names in the namespace methods.
Currently, these ops live in the image namespace together with such diverse things as ascii, distancetransform, and histogram. I think this was discussed already, but would it make sense to create nested namespaces inside image, or to split them into other namespaces instead?
Good point as well, but definitely something we can discuss afterward.
This would be amazing, though I fear that the information used in the Op Finder is extracted from the inherited instance variables
in1
andin2
. I did at least change the names in the namespace methods.
I had a quick chat with @ctrueden about this and scijava-ops
will allow for class-based @Parameter
annotations for in1
and in2
. See ParameterTest.java#L197-L198
for an example of how it could look like.
@stelfrich Thanks for your work on this! I modified the name of your unit tests, so maven runs them now. And have suggested a few minor edits.
I had a quick chat with @ctrueden about this and
scijava-ops
will allow for class-based@Parameter
annotations forin1
andin2.
SeeParameterTest.java#L197-L198
for an example of how it could look like.
I don't know if we want to wait until this is resolved before merging this, or if we're OK with merging this without? Basically it is to rename/annotate in1
and in2
right? I think for now renaming them in the namespace methods is sufficient. Thoughts? @ctrueden? @imagejan?
I think after the minor edits I suggested, this should be good to merge. But if you object please let me know. 😺
I don't know if we want to wait until this is resolved before merging this, or if we're OK with merging this without?
If we wait, we will be waiting for at least a few months. I'd say we should merge before that. Normally, I like the namespace method variable names to match the parameter fields, but in the case of in
/in1
/in2
/out
I am OK with naming them differently, with the idea that when Ops migrates to the new SJC framework, the names from the namespace methods will serve as a hint for what to name the new @Parameter
declarations.
Thanks for your comments, @awalter17! They have been addressed in two separate commits.
I think after the minor edits I suggested, this should be good to merge.
I'd say we should merge before that.
Feel free to merge! 👍