Orange ImageAnalytics client updated to repeat sending images when request not successful #66

PrimozGodec · 2017-10-12T10:56:17Z

Issue

Image Embedding client is sometimes rejected by a server due to too many requests and some other reasons, what means that that it does not retrieve embeddings for some images.

Description of changes

The client was changed that way that images which are rejected are sent to the a again until the client gets their embeddings or a maximum number of attempts is reached.

Includes

Code changes
Tests
Documentation

…essful

codecov-io · 2017-10-12T10:59:08Z

Codecov Report

Merging #66 into master will decrease coverage by 2.1%.
The diff coverage is 46.87%.

@@            Coverage Diff             @@
##           master      #66      +/-   ##
==========================================
- Coverage   58.53%   56.42%   -2.11%     
==========================================
  Files           5        6       +1     
  Lines         422      475      +53     
  Branches       65       79      +14     
==========================================
+ Hits          247      268      +21     
- Misses        161      191      +30     
- Partials       14       16       +2

PrimozGodec · 2017-11-03T09:45:11Z

Can somebody test that PR? @inejc @astaric @jerneju I was testing that with yeast dataset https://www.dropbox.com/sh/a5h3yp740bpovjf/AABIDnO9Po-xu8xCXoXgO7t5a?dl=0. Also use some other images. In general all images that are not damaged should get embeddings (no none values).

matjazp · 2017-11-03T09:54:23Z

orangecontrib/imageanalytics/image_embedder.py


    def __init__(self, model="inception-v3", layer="penultimate",
-                 server_url='api.biolab.si:8080'):
+                 server_url='api.garaza.io:443'):


Maybe it's better to switch to a different URL with a separate PR, and not add it here?
Note that this URL is also temporary and will probably change as soon as we promote the new test k8s cluster into a production.

kernc · 2017-11-03T12:31:20Z

orangecontrib/imageanalytics/hyper/stream.py

+# -*- coding: utf-8 -*-
+"""
+Temporary fix for hyper library until next version from 0.7.0 is out
+"""


This file is under MIT license from hyper.

Another thing to consider: development branch of hyper has a lot of new commits after the last version 0.7.0 (see python-hyper/hyper@v0.7.0...development). You are fixing just one bug. Maybe it would be better to base our usage on development branch and not on 0.7.0?

In the mean time, as we're waiting for upstream to merge python-hyper/hyper#356, we can use in requirements.txt your fixed branch
e.g.

pip install git+https://github.com/PrimozGodec/hyper@d8ed47aadbc1b66e8493726beeffcf0361ae8729

We switch to the upstream's version as soon the fix is merged. And nudge Lukasa to make another official version ;)

If we plan to release a version of image-analytics add-on with this fix, I would rather have it base on a released version of the hyper package.

Otherwise, we are installing development version of hyper package into users environment, If we distribute over conda, we need to make sure hyper-devel builds are available, ...

Ok if we are waiting for new release do I use my fixed branch anyway or leave as it is?

@astaric I am not sure if we want to wait until next release. Hyper had a last release 0.7.0 more than one year ago. What means that releases are not so frequent. Is there any other solution since we are in hurry in margin this?

I agree. I've asked to make e new release, but I don't know when it will be done. We can wait few days. Otherwise, we can just use a fixed commit ID in hyper and upgrade later?

git+https://github.com/Lukasa/hyper@a8109c3aaf8e2aa7314f23bf46e20af2bc241cd7

Feel free to use git+... in requirements and merge this PR.

But please do not create a (pypi) release that depends on development versions of packages.

BlazZupan · 2017-11-04T05:54:50Z

DeepLoc is currently the default. Its performance is poor (even classification accuracy on yeast localization dataset). Instead, let the default be Inception v3 and let the order of the embedders be

Inception v3 (default), VGG-16, VGG-19, Painters, DeepLoc

VGG-19 does not work, the widget reports the following error:

Traceback (most recent call last):
File "/Users/blaz/Documents/dev/orange3-imageanalytics/orangecontrib/imageanalytics/http2_client.py", line 111, in _get_json_response_or_none
return json.loads(response_txt)
File "/usr/local/Cellar/python3/3.6.2/Frameworks/Python.framework/Versions/3.6/lib/python3.6/json/init.py", line 354, in loads
return _default_decoder.decode(s)
File "/usr/local/Cellar/python3/3.6.2/Frameworks/Python.framework/Versions/3.6/lib/python3.6/json/decoder.py", line 339, in decode
obj, end = self.raw_decode(s, idx=_w(s, 0).end())
File "/usr/local/Cellar/python3/3.6.2/Frameworks/Python.framework/Versions/3.6/lib/python3.6/json/decoder.py", line 357, in raw_decode
raise JSONDecodeError("Expecting value", s, err.value) from None
json.decoder.JSONDecodeError: Expecting value: line 1 column 1 (char 0)

BlazZupan · 2017-11-04T06:08:38Z

The widget displays Cancel button when working. This changes the size of the window. It would be better if the Cancel button would be displayed all the time, but disabled when the widget is not working. Such that the widget would not resize.

matjazp · 2017-11-04T17:26:14Z

I've updated VGG19. The problem was that it takes 7-8 sec/req and the default timeout for proxy_read was set at the standard 60s (nginx waits 60s for a response, then closes the connection). I've increased the timeout to 5 min. However, the same thing WILL happen again when the load will increase beyond a certain point.

Server actually correctly returned 504 Gateway Timeout, so the client should be more robust with this kind of errors with an appropriate messages to the user. Maybe even wait a bit before resending a request in this situation as this will only make things worse.

astaric · 2017-11-06T07:58:57Z

Could you add a link to the upstream fix on hyper?

matjazp · 2017-11-06T08:07:49Z

@astaric it is tracked in python-hyper/hyper#356

PrimozGodec · 2017-11-09T09:24:24Z

python-hyper/hyper#356 is now merged

PrimozGodec · 2017-11-10T10:09:09Z

I think it is ready to merge. Can somebody review the code and merge ASAP? @astaric @jerneju

matjazp · 2017-11-10T10:53:39Z

@PrimozGodec I think it would be better to just fix requirements.txt with a specific version

git+https://github.com/Lukasa/hyper@a8109c3aaf8e2aa7314f23bf46e20af2bc241cd7

In that case you don't even need to add your stream.py and all we have to do after a new release of Hyper is to just use a released version in requirements.txt.

astaric · 2017-11-10T11:01:59Z

@matjazp and solve all the problems users that already have hyper installed will have once we replace it with an unreleased version. Or when they want to install software with conda, which does not allow installation from git (at least not that I am aware of).

PrimozGodec added 4 commits October 12, 2017 10:58

Image embedder client repeats sending request to server when not succ…

d486b86

…essful

Skipped images that are not loaded for future resending

d6f33eb

Updated advance function to advance only when successful

170f033

Number of repeats of sending image to the server set to 4

3a7e746

PrimozGodec changed the title ~~Robust client~~ Orange ImageAnalytics client updated to repeat sending images when request not successful Oct 12, 2017

PrimozGodec changed the title ~~Orange ImageAnalytics client updated to repeat sending images when request not successful~~ [WIP] Orange ImageAnalytics client updated to repeat sending images when request not successful Oct 20, 2017

PrimozGodec added 3 commits October 23, 2017 14:34

Registered new embedders

fcd99f4

Url changed to new service

941af93

Temporary fix for hyper library

8ca4fc7

PrimozGodec changed the title ~~[WIP] Orange ImageAnalytics client updated to repeat sending images when request not successful~~ Orange ImageAnalytics client updated to repeat sending images when request not successful Nov 3, 2017

matjazp reviewed Nov 3, 2017

View reviewed changes

kernc reviewed Nov 3, 2017

View reviewed changes

PrimozGodec added 2 commits November 6, 2017 13:58

Changed embedders order and default

230e5fb

Cancel button always on just disabled when no actual.

f20514e

Local fix for hyper updated to hyper fix

b247052

astaric merged commit e721e17 into biolab:master Nov 10, 2017

PrimozGodec deleted the robust-client branch November 10, 2017 13:29

This was referenced Nov 10, 2017

ImageEmbedder forcefully closes stream #61

Closed

ImageEmbedder - Memory Error #64

Closed

Uh oh!

Orange ImageAnalytics client updated to repeat sending images when request not successful #66

Orange ImageAnalytics client updated to repeat sending images when request not successful #66

Uh oh!

Conversation

PrimozGodec commented Oct 12, 2017

Issue

Description of changes

Includes

Uh oh!

codecov-io commented Oct 12, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

PrimozGodec commented Nov 3, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

BlazZupan commented Nov 4, 2017

Uh oh!

BlazZupan commented Nov 4, 2017

Uh oh!

matjazp commented Nov 4, 2017

Uh oh!

astaric commented Nov 6, 2017

Uh oh!

matjazp commented Nov 6, 2017

Uh oh!

PrimozGodec commented Nov 9, 2017

Uh oh!

PrimozGodec commented Nov 10, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

matjazp commented Nov 10, 2017

Uh oh!

astaric commented Nov 10, 2017

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

codecov-io commented Oct 12, 2017 •

edited

Loading

PrimozGodec commented Nov 3, 2017 •

edited

Loading

PrimozGodec commented Nov 10, 2017 •

edited

Loading