Use better cd_values[] in LZ4 example

[ajelenak]

The old value does not seem appropriate for the LZ4 filter as it is too small. Such small values will create much larger compressed chunks than the original chunk size.

[byrnHDF]

Because this change comes from a branch in a fork, the reqular plugin workflow fails to run, the daily-build workflow needs to run from the fork and use the ignore option.

[ajelenak]

the daily-build workflow needs to run from the fork and use the ignore option.

@byrnHDF https://github.com/HDFGroup/hdf5_plugins/actions/runs/15354926926

[byrnHDF]

Right so you need to update the testfiles references to match the output of the tests

[ajelenak]

It seems more than just that. The example fails on this line:

hdf5_plugins/LZ4/src/H5Zlz4.c

Line 156 in 5c28b5b

printf("decompressed size not the same: %d, != %d\n", compressedBytes, compressedBlockSize);

with the error:

decompressed size not the same: -81, != 96

[byrnHDF]

Yes, that does indicate a coding error.

[byrnHDF]

Changing the case will also need to be applied to the examples in HDF5 repo.

[ajelenak]

Are you okay with this capitalization change @byrnHDF ? I wanted to ask you but you responded faster.

[byrnHDF]

Was there a reason - because it will have consequences in other projects. Also is it consistent with the other filters - or will those need changed as well. Allen

…

________________________________________ From: Aleksandar Jelenak ***@***.***> Sent: Wednesday, June 4, 2025 11:57 AM To: HDFGroup/hdf5_plugins Cc: Allen Byrne; Mention Subject: Re: [HDFGroup/hdf5_plugins] Use better cd_values[] in LZ4 example (PR #186) [https://avatars.githubusercontent.com/u/7267124?s=20&v=4]ajelenak left a comment (HDFGroup/hdf5_plugins#186)<#186 (comment)> Are you okay with this capitalization change @byrnHDF<https://github.com/byrnHDF> ? I wanted to ask you but you responded faster. — Reply to this email directly, view it on GitHub<#186 (comment)>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/AL77KBWKHCIVKWHZQBE6FAL3B4QPBAVCNFSM6AAAAAB6G5OHRGVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDSNBQG42TIMJTGY>. You are receiving this because you were mentioned.Message ID: ***@***.***>

[ajelenak]

Was there a reason - because it will have consequences in other projects.

Projects out there, or in-house, like HDF5?

The reason was the name of the filter is LZ4, not lz4. It is not a required change. Now that I better understand how many places need to be changed, I will switch back.

[byrnHDF]

Yes, usually my choice too, if it involves a lot more work for little gain.

…

________________________________________ From: Aleksandar Jelenak ***@***.***> Sent: Wednesday, June 4, 2025 1:28 PM To: HDFGroup/hdf5_plugins Cc: Allen Byrne; Mention Subject: Re: [HDFGroup/hdf5_plugins] Use better cd_values[] in LZ4 example (PR #186) [https://avatars.githubusercontent.com/u/7267124?s=20&v=4]ajelenak left a comment (HDFGroup/hdf5_plugins#186)<#186 (comment)> Was there a reason - because it will have consequences in other projects. Projects out there, or in-house, like HDF5? The reason was the name of the filter is LZ4, not lz4. It is not a required change. Now that I better understand how many places need to be changed, I will switch back. — Reply to this email directly, view it on GitHub<#186 (comment)>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/AL77KBTPCHGXX23IF7VIEFL3B43D3AVCNFSM6AAAAAB6G5OHRGVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDSNBQHE4TQMJYGQ>. You are receiving this because you were mentioned.Message ID: ***@***.***>

[ajelenak]

Ok, I removed those changes. Back to the important stuff...

I switched to an older version of the LZ4 plugin in this branch, and it worked for a more sensible block size in the LZ4 example. Also all LZ4-related tests are updated accordingly. We now need to decide how to proceed. It would be nice to understand why introduction of the LZ4_decompress_safe() function in the LZ4 plugin is not working with a block size larger than 3 bytes but also it is important to have a really working LZ4 plugin in this repository.

[ajelenak]

The LZ4 plugin seems to work now using the new LZ4_decompress_safe() function. The updated LZ4 example code does not report an error about decompressed block size difference.

[nhz2]

LZ4_decompress_safe existed before LZ4_VERSION_NUMBER existed lz4/lz4@64547df

So I think the version checks can be safely removed.

[ajelenak]

Yes, it would simplify the code. I am not familiar with the LZ4 evolution, is there any chance that older versions of this function were slower than the current implementation?

[nhz2]

Yes, but even for older versions of LZ4, the better error handling is worth it.

[nhz2]

Looks good

[dkokron]

I am eager to test LZ4 compression in NetCDF Operators (NCO) and NetCDF itself. What steps remain before this PR can be merged?

[ajelenak]

All LZ4 tests pass. Fixed #185.