Skip to content
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.

Sign up for GitHub

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

Jump to bottom

NTNDArray decompression improvements #4

Closed
MarkRivers opened this issue Dec 3, 2018 · 33 comments
Closed

NTNDArray decompression improvements #4

MarkRivers opened this issue Dec 3, 2018 · 33 comments
Milestone
R1-4

Comments

@MarkRivers
Copy link
Member

@mrkraimer has written a new NTNDCodec.java which moves the decompression code into a separate class. This is cleaner and more re-usable so it should be used in a future release. It will require some work, and I want to release R1-3 soon so this will wait for R1-4.
@MarkRivers MarkRivers added this to the R1-4 milestone Dec 3, 2018
@mrkraimer
Copy link
Contributor

There are problem so do not merge branch ntndcodec

@MarkRivers
Copy link
Member Author

I am working today to implement 2 new decompressors, LZ4 and BitshuffleLZ4. I can either do this in the master branch, or in the ntndcodec branch. If I do it in the master branch there will be work to do to move those new functions into the new NTNDCodec class.

What are the problems with the NTNDCodec branch? Should I work on fixing those, or just work on the master branch?

@mrkraimer
Copy link
Contributor

I just merged my changes.
blosc should now work.
I upgraded to feroda 29 and now the code for jpeg can not be found.

Go ahead an make your changes on branch ntndcodec.
Let me know when You have pushed your changes.

@mrkraimer
Copy link
Contributor

Mark,
I see that you have made the changes on the master branch.
When you are done please let me know and I will make similar changes to ntndcodec branch.
I assume that I also have to get the latest changes in ADCore.
Are there any other components of areaDetector that I need to update?

@MarkRivers
Copy link
Member Author

MarkRivers commented Dec 9, 2018 via email

@MarkRivers
Copy link
Member Author

I have merged my changes. The changes are:

  • Added support for lz4 and bitshuffle/lz4.
  • Improved the logic. The large "if else" block for each datatype was previously only for Blosc. Now it works on each of the compressors. Decompression is done before this block for each compression type.

I have a Java question. I tried changing this statement:

        if (scalarType==ScalarType.pvByte) {

to

    switch(scalarType) {
        case ScalarType.pvByte:

        case ScalarType.pvUByte:

etc.

But the Java compiler complains that these are not enums I am comparing to?

Should we be distributing a .jar file or just the .java files? I just discovered that if I delete all of the .class files in the ImageJ/plugins/EPICS_areaDetector directory, then I only need to do Compile and Run on EPICS_NTNDA_Viewer.java. It automatically finds and compiles all of the other .java files required (myUtil.java, decompressJPEGDll.java, etc.). This is convenient, and means perhaps we don't really need a .jar file, since the user only has to compile a single file.

@MarkRivers
Copy link
Member Author

I merged the ntndcodec branch into master and deleted it on Github.

I tested it on both Linux and Windows.

@mrkraimer
Copy link
Contributor

mrkraimer commented Dec 10, 2018 via email

@MarkRivers
Copy link
Member Author

I think I see your JPEG problem.

I just ran "ldd" on ADSupport/lib/linux-x86_64/libdecompressJPEG.so. I see the following:

corvette:ADSupport/lib/linux-x86_64>ldd libdecompressJPEG.so
        linux-vdso.so.1 =>  (0x00007ffd659fe000)
        libjpeg.so => /lib64/libjpeg.so (0x00007f951d12c000)
        libpthread.so.0 => /lib64/libpthread.so.0 (0x00007f951cf0f000)
        librt.so.1 => /lib64/librt.so.1 (0x00007f951cd07000)
        libdl.so.2 => /lib64/libdl.so.2 (0x00007f951cb03000)
        libstdc++.so.6 => /lib64/libstdc++.so.6 (0x00007f951c7fb000)
        libm.so.6 => /lib64/libm.so.6 (0x00007f951c4f9000)
        libgcc_s.so.1 => /lib64/libgcc_s.so.1 (0x00007f951c2e3000)
        libc.so.6 => /lib64/libc.so.6 (0x00007f951bf15000)
        /lib64/ld-linux-x86-64.so.2 (0x000055e2747e3000)

Note that it depends on /lib64/libjpeg.so. That is not correct, because it means you need to have the jpeg devel package installed. It should be using the version in ADSupport, not the system version. I will look into it.

@MarkRivers
Copy link
Member Author

I found that on my system it was finding the system version of libjpeg.so because in that terminal I had not defined LD_LIBRARY_PATH to include ADSupport/lib/linux-x86_64. Once I did that then ldd reported this:

corvette:ADSupport/lib/linux-x86_64>ldd libdecompressJPEG.so
        linux-vdso.so.1 =>  (0x00007fffb593a000)
        libjpeg.so => /home/epics/devel/areaDetector/ADSupport/lib/linux-x86_64/libjpeg.so (0x00007f969c3e0000)
        libpthread.so.0 => /lib64/libpthread.so.0 (0x00007f969c1a7000)
        librt.so.1 => /lib64/librt.so.1 (0x00007f969bf9f000)
        libdl.so.2 => /lib64/libdl.so.2 (0x00007f969bd9b000)
        libstdc++.so.6 => /lib64/libstdc++.so.6 (0x00007f969ba93000)
        libm.so.6 => /lib64/libm.so.6 (0x00007f969b791000)
        libgcc_s.so.1 => /lib64/libgcc_s.so.1 (0x00007f969b57b000)
        libc.so.6 => /lib64/libc.so.6 (0x00007f969b1ad000)
        /lib64/ld-linux-x86-64.so.2 (0x00005649f0042000)

What do you see when you run ldd on libdecompressJPEG.so?

@MarkRivers
Copy link
Member Author

Regarding the byte order issue, I think we really need to test to be sure.

Both the Blosc compressor and the bitshuffle compressor are passed the number of bytes per element. Each of them puts a header at the start of the compressed block. It is possible that that header contains both the number of bytes per element, and the endianess of the machine that did the compression. The decompressor would then have the information needed to do the byte swapping to the endianess of the decompressing machine. I don't know if either decompressor actually does this, but in principle they might.

For straight lz4 compression the information on bytes per element is not provided to the compressor or decompressor, so I don't see how it could do byte swapping.

The issue is not too pressing, because most of the machines we are using for these large arrays are currently Intel little-endian. But eventually we should get it right, but that means finding big-endian machines to test with.

@mrkraimer
Copy link
Contributor

mrkraimer commented Dec 10, 2018 via email

@mrkraimer
Copy link
Contributor

mrkraimer commented Dec 10, 2018 via email

@MarkRivers
Copy link
Member Author

For the JPEG problem try temporarily coping libdecompressJPEG.so into ImageJ/plugins/EPICS_areaDetector.

When I was having a problem I found that when I did that I got a better diagnostic error message about the real problem.

Be sure to check both the ImageJ log window and your Linux shell Window that launched ImageJ for error messages.

@mrkraimer
Copy link
Contributor

mrkraimer commented Dec 10, 2018 via email

@MarkRivers
Copy link
Member Author

If you had not set LD_LIBRARY_PATH then how did ImageJ find the libraries for bitshuffle and Blosc? Perhaps those come with the new Fedora, and so it was using the system versions?

@mrkraimer
Copy link
Contributor

mrkraimer commented Dec 10, 2018 via email

@MarkRivers
Copy link
Member Author

Hi Marty,
There was no attachment, so I don't know what the problem was. For Github messages you need to log in, and use the Issue to reply. You can then drag or paste images into the message.

@MarkRivers
Copy link
Member Author

I did a commit in NTNDCodec.java at 21:23 yesterday. Did you get that? It was causing bitshuffle and lz4 not to work. The problem was with the variable numElements, which was causing confusion. It was being used to mean both "the number of elements in imagedata", and "the size of imagedata in bytes", which are not the same for non-byte data. I removed the variable completely since it's simpler.

@mrkraimer
Copy link
Contributor

mrkraimer commented Dec 11, 2018 via email

@mrkraimer
Copy link
Contributor

Sorry I spoke too soon.
I still see the error on
image

@mrkraimer mrkraimer reopened this Dec 11, 2018
@MarkRivers
Copy link
Member Author

Hi Marty,

That error means that WITH_BITSHUFFLE=YES is not set in your areaDetector/configure/CONFIG_SITE.local. That was added to EXAMPLE_CONFIG_SITE.local the other day.

@mrkraimer
Copy link
Contributor

mrkraimer commented Dec 11, 2018 via email

@MarkRivers
Copy link
Member Author

And if I do not set LD_LIBRARY_PATH all of JPEG, LZ4, and BSLZ4 fail because the code can not be found

OK, good that makes sense.

@mrkraimer
Copy link
Contributor

mrkraimer commented Dec 11, 2018 via email

@MarkRivers
Copy link
Member Author

JBlosc is just a Java wrapper around the Blosc C library, using com.sun.jna. Since JBlosc exists, why not use it instead of writing our own?

@mrkraimer
Copy link
Contributor

mrkraimer commented Dec 11, 2018 via email

@MarkRivers
Copy link
Member Author

To make sure that what appears in ADSupport is in sync with what is in ADViewers.

That is already guaranteed. Jblosc is using the blosc C library from ADSupport. Jblosc does not have its own copy of the C library.

@MarkRivers
Copy link
Member Author

That is already guaranteed. Jblosc is using the blosc C library from ADSupport. Jblosc does not have its own copy of the C library.

Or are you worried about something different?

@MarkRivers MarkRivers reopened this Dec 11, 2018
@mrkraimer
Copy link
Contributor

mrkraimer commented Dec 12, 2018 via email •
edited
Loading

@MarkRivers
Copy link
Member Author

I have taken your advance and removed jblosc. I added decompressBloscDll.java to replace it.

I renamed myUtil.java to ByteBufferUtil.java, and removed its dependency on Jblosc.

I changed NTNDCodec.java to use switch statements rather than long if else if blocks. I think it makes it easier to read.

Thanks for the help and advice.

@mrkraimer
Copy link
Contributor

You are welcome.
Also please delete branch ntndcodec

@MarkRivers
Copy link
Member Author

I have deleted the ntndcodec branch. Closing this issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
R1-4
Development

No branches or pull requests
2 participants
@MarkRivers @mrkraimer