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.

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

Docker ci #245

Merged
merged 12 commits into from Jan 4, 2022
Merged

Docker ci #245

merged 12 commits into from Jan 4, 2022

Conversation

jacob-rosenthal
Copy link
Collaborator

@jacob-rosenthal jacob-rosenthal commented Dec 6, 2021

Add a Dockerfile which builds a working environment for pathml and starts up a jupyterlab instance in the container, which users can connect to and get up and running quickly. Also add a github actions workflow to build the image and publish it to dockerhub whenever we create a new release

This will close #145

@jacob-rosenthal
Copy link
Collaborator Author

@dmbrundage @ryanccarelli Can you take a look and let me know what you think? Maybe test out the container yourself to see if it works for you, and if the instructions are clear enough?

@codecov-commenter
Copy link

codecov-commenter commented Dec 6, 2021

Codecov Report

Merging #245 (fb81faf) into master (2721cb7) will decrease coverage by 0.05%.
The diff coverage is 77.77%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #245      +/-   ##
==========================================
- Coverage   84.69%   84.63%   -0.06%     
==========================================
  Files          26       26              
  Lines        2430     2434       +4     
==========================================
+ Hits         2058     2060       +2     
- Misses        372      374       +2     
Impacted Files Coverage Δ
pathml/core/slide_backends.py 96.16% <75.00%> (-0.60%) ⬇️
pathml/_version.py 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 2721cb7...fb81faf. Read the comment docs.

@dmbrundage
Copy link
Contributor

Thanks @jacobrosenthaldfci I will take a look. Does it require you to have pathml locally to copy over? Also is there an image pushed to docker hub that can be pulled?

@jacob-rosenthal
Copy link
Collaborator Author

Yes, the way it is set up is that you can build the docker container from a local clone of the repo - or, we will also publish a container to docker hub so you can just do docker pull (that's what the github action should do)

@dmbrundage
Copy link
Contributor

Great! I just built it locally and can confirm that I was able to launch Jupyter Lab and load PathML. Instructions were perfectly clear, not sure if there needs to be a call out to copy the token or URL from the docker CLI.

@jacob-rosenthal
Copy link
Collaborator Author

Ok great thanks David! I guess we need to test the github action now. Once we merge this to master, I'll make a new release so we can verify that the image is properly built and uploaded to docker hub.

@jacob-rosenthal jacob-rosenthal marked this pull request as ready for review December 7, 2021 15:29
Copy link
Contributor

@ryanccarelli ryanccarelli left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will be so convenient to have. I found an issue with Java/Javabridge:

from pathml import VectraSlide, types
test = VectraSlide("/home/anokhiml/Documents/pathml/tests/testdata/small_vectra.qptiff", slide_type = types.IF)
---------------------------------------------------------------------------
JavaException                             Traceback (most recent call last)
/tmp/ipykernel_16/3376390402.py in <module>
      1 from pathml import VectraSlide, types
----> 2 test = VectraSlide("/home/anokhiml/Documents/pathml/tests/testdata/small_vectra.qptiff", slide_type = types.IF)

/usr/local/lib/python3.8/site-packages/pathml/core/slide_data.py in __init__(self, *args, **kwargs)
    501         if "backend" not in kwargs:
    502             kwargs["backend"] = "bioformats"
--> 503         super().__init__(*args, **kwargs)
    504 
    505 

/usr/local/lib/python3.8/site-packages/pathml/core/slide_data.py in __init__(self, filepath, name, masks, tiles, labels, backend, slide_type, stain, platform, tma, rgb, volumetric, time_series, counts)
    174             backend_obj = pathml.core.OpenSlideBackend(filepath)
    175         elif backend.lower() == "bioformats":
--> 176             backend_obj = pathml.core.BioFormatsBackend(filepath)
    177         elif backend.lower() == "dicom":
    178             backend_obj = pathml.core.DICOMBackend(filepath)

/usr/local/lib/python3.8/site-packages/pathml/core/slide_backends.py in __init__(self, filename)
    246         # init java virtual machine
    247         javabridge.start_vm(class_path=bioformats.JARS, max_heap_size="50G")
--> 248         _init_logger()
    249         # java maximum array size of 2GB constrains image size
    250         ImageReader = bioformats.formatreader.make_image_reader_class()

/usr/local/lib/python3.8/site-packages/pathml/core/slide_backends.py in _init_logger()
    224         "ch/qos/logback/classic/Level", "WARN", "Lch/qos/logback/classic/Level;"
    225     )
--> 226     javabridge.call(
    227         rootLogger, "setLevel", "(Lch/qos/logback/classic/Level;)V", logLevel
    228     )

/usr/local/lib/python3.8/site-packages/javabridge/jutil.py in call(o, method_name, sig, *args)
    886     '''
    887     env = get_env()
--> 888     fn = make_call(o, method_name, sig)
    889     args_sig = split_sig(sig[1:sig.find(')')])
    890     ret_sig = sig[sig.find(')')+1:]

/usr/local/lib/python3.8/site-packages/javabridge/jutil.py in make_call(o, method_name, sig)
    849     if method_id is None:
    850         if jexception is not None:
--> 851             raise JavaException(jexception)
    852         raise JavaError('Could not find method name = "%s" '
    853                         'with signature = "%s"' % (method_name, sig))

JavaException: setLevel

@jacob-rosenthal
Copy link
Collaborator Author

Good catch, thanks Ryan!
Not sure why this is happening on the docker image but not otherwise... my only guess is that it could be related to the difference in java installations? We usually use the openjdk8 conda package, but in the docker container we use adoptopenjdk-8-hotspot.
Added a try-except block to catch this error - users of the docker image will just get some extra logs which I think isn't a big problem

@ryanccarelli
Copy link
Contributor

Great! If tests pass when run from inside the container and we can get through H&E and MIF pipelines then I think it's good to go

@jacob-rosenthal
Copy link
Collaborator Author

ok having some more trouble with javabridge inside the container...

First I entered the container with the tests folder mounted: docker run -v "$(pwd)/tests:/opt/pathml/tests" -it -p 8888:8888 pathml-analysis

Then when running tests, I'm getting lots of failures,:

# python -m pytest -m "not slow" -x
============================= test session starts ==============================
platform linux -- Python 3.8.12, pytest-6.2.5, py-1.11.0, pluggy-1.0.0
rootdir: /opt/pathml
plugins: anyio-3.4.0
collected 443 items / 1 deselected / 442 selected                              

tests/test_utils.py ...................                                  [  4%]
tests/test_version.py .                                                  [  4%]
tests/core_tests/test_h5managers.py ..                                   [  4%]
tests/core_tests/test_masks.py ..................                        [  9%]
tests/core_tests/test_slide_backends.py .F

=================================== FAILURES ===================================
_______________ test_extract_region[None-50-location0-backend1] ________________

backend = BioFormatsBackend('tests/testdata/smalltif.tif'), location = (0, 0)
size = 50, level = None

    @pytest.mark.parametrize(
        "backend", [openslide_backend(), bioformats_backend(), bioformats_backend_qptiff()]
    )
    @pytest.mark.parametrize("location", [(0, 0), (50, 100)])
    @pytest.mark.parametrize("size", [50, (50, 100)])
    @pytest.mark.parametrize("level", [None, 0])
    def test_extract_region(backend, location, size, level):
>       region = backend.extract_region(location=location, size=size, level=level)

tests/core_tests/test_slide_backends.py:39: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 
pathml/core/slide_backends.py:346: in extract_region
    slicearray = reader.read(
/usr/local/lib/python3.8/site-packages/bioformats/formatreader.py:904: in read
    image = np.frombuffer(openBytes_func(index),dtype)
/usr/local/lib/python3.8/site-packages/bioformats/formatreader.py:784: in <lambda>
    openBytes_func = lambda x: self.rdr.openBytesXYWH(x, XYWH[0], XYWH[1], XYWH[2], XYWH[3])
/usr/local/lib/python3.8/site-packages/javabridge/jutil.py:961: in method
    result = call(self.o, name, sig, *args)
/usr/local/lib/python3.8/site-packages/javabridge/jutil.py:892: in call
    result = fn(*nice_args)
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 

args = (1, 0, 0, 50, 50), result = None, x = <Java object at 0x-17a44c8>

    def fn(*args):
        result = env.call_method(o, method_id, *args)
        x = env.exception_occurred()
        if x is not None:
>           raise JavaException(x)
E           javabridge.jutil.JavaException: <Java object at 0x-17a44c8>

/usr/local/lib/python3.8/site-packages/javabridge/jutil.py:859: JavaException
----------------------------- Captured stderr call -----------------------------
Exception in thread "Thread-0" java.lang.ArrayIndexOutOfBoundsException
        at java.lang.System.arraycopy(Native Method)
        at loci.formats.tiff.TiffParser.getSamples(TiffParser.java:1087)
        at loci.formats.tiff.TiffParser.getSamples(TiffParser.java:834)
        at loci.formats.in.MinimalTiffReader.openBytes(MinimalTiffReader.java:306)
        at loci.formats.DelegateReader.openBytes(DelegateReader.java:227)
        at loci.formats.FormatReader.openBytes(FormatReader.java:886)
=============================== warnings summary ===============================
../../usr/local/lib/python3.8/site-packages/spams_wrap/spams_wrap.py:19
  /usr/local/lib/python3.8/site-packages/spams_wrap/spams_wrap.py:19: DeprecationWarning: the imp module is deprecated in favour of importlib; see the module's documentation for alternative uses
    import imp

tests/core_tests/test_slide_backends.py:173
  /opt/pathml/tests/core_tests/test_slide_backends.py:173: PytestUnknownMarkWarning: Unknown pytest.mark.slow - is this a typo?  You can register custom marks to avoid this warning - for details, see https://docs.pytest.org/en/stable/mark.html
    @pytest.mark.slow

-- Docs: https://docs.pytest.org/en/stable/warnings.html
=========================== short test summary info ============================
FAILED tests/core_tests/test_slide_backends.py::test_extract_region[None-50-location0-backend1]
!!!!!!!!!!!!!!!!!!!!!!!!!! stopping after 1 failures !!!!!!!!!!!!!!!!!!!!!!!!!!!
=========== 1 failed, 41 passed, 1 deselected, 2 warnings in 12.18s ============
# 

It's hard for me to tell exactly what's going on but I'm guessing it's also connected to using the different java version or something... will continue looking into this

@jacob-rosenthal
Copy link
Collaborator Author

Will try out building from openjdk:8 base image, and installing python on top (instead of currently starting with a python image and installing openjdk8 on top)

@jacob-rosenthal
Copy link
Collaborator Author

This Docker image contains pathml and all dependencies (including jdk8, deepcell, etc.). It also runs the full test suite in the build process to make sure everything is working as expected.

Version number is 2.0.dev1 so that we can merge to master and test out the Github action which should build the image and publish it to Dockerhub. Once we verify that's working, we can bump to version 2.0.2

@jacob-rosenthal jacob-rosenthal merged commit 777deed into master Jan 4, 2022
@jacob-rosenthal jacob-rosenthal deleted the docker-ci branch February 13, 2022 18:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Set up github actions for packaging/distribution
4 participants