Fix JPEG 2000 float->int quantization#26
Conversation
|
Though we also have to keep in mind that we already introduce error during quantization so the PSNR bound likely needs to be a bit lower to compensate |
|
Thanks @juntyr ! I will try to convert it into a working version tomorrow. I also looked at SZ3 and they seem to introduce a rather cryptic adjustment factor when converting from PSNR->Absolute Error where the |
|
SZ3 goes the other way around where it translates everything into a pointwise absolute error (so quite strict), but I also don't fully understand their conversions |
Well we know what absolute error linear quantisation incurs and if we assume that worst case every element incurs it, we could include that in the absolute error, right? |
The error for linear quantisation should provide a lower bound of what absolute error we can achieve, right? So we might need a warning if the user provides a lower bound that is below the error incurred due to the quantization. Also thinking about the quantization a bit more. I think we need to adjust the PSNR calculation that we are passing to the JPEG compressor. Because JPEG2000 will compute the PSNR on the linearly quantized data, right? Because the input data and quantized data are on different scales the PSNR values will change as well. Does that make sense @juntyr ? |
|
So I pushed a committ that makes the code run, it actually only required a minor change in the actual code. But right now this code leads to the following error (the same error occurs on multiple datasets, CAMS and ERA5): I'm a bit stuck on debugging this error. I went into the debugger and checked the behaviour of all the elements in the |
| def abs_bound_codec( | ||
| error_bound, | ||
| *, | ||
| data_abs_min=None, |
There was a problem hiding this comment.
The JPEG codec needs the actual minimum and maximum, not the minimum absolute value.
There was a problem hiding this comment.
Ah sorry, yes that's a bug. It won't be the cause of the issue though because for CAMS all the values are positive.
|
Could you check again that the quantized elements are indeed in range? The error is only raised for uint32 if the value is > (2**25 - 1) |
|
This is the check I have done: with the breakpoint set at |
|
Having another stab at figuring out what could be going on here..
I thought about this again and actually I was wrong, we don't need to adjust the PSNR because it is invariant to the scale-shift operations that we are doing. |
Additionally, adjusted the new compressors that came from the main branch to be able to work with the new abstract base class interface.
|
@juntyr So setting Either way, should be ready to merge now! |
| # round and truncate to integer values | ||
| numcodecs_wasm_round.Round(precision=1), | ||
| numcodecs.astype.AsType( | ||
| encode_dtype="int32", |
There was a problem hiding this comment.
That's the problem, you're right. We're going to signed int32, which supports [-2^24, 2^24 - 1]. If you change this to uint32 (since we now scale to [0, ...]), the max pixel value of 2^25 - 1 should work.
There was a problem hiding this comment.
Just checked it, you're right! I know switched back to max pixel val 2^25 - 1 but using uint32 because it should give more range!
There was a problem hiding this comment.
Thank you for finding what our issue was!
While experimenting, I realized that our current float->int remapping for JPEG 2000 is faulty.
This commit doesn't work by itself since building the compressor now requires extra info on the range. Perhaps @treigerm could take a look on how to get it actually working? Feel free to push to this branch.
Fixes #18