Scan-o-Matic / scanomatic

Scanomatic
GNU General Public License v3.0
10 stars 4 forks source link

Set griding outputs are confusin or dont match #133

Closed Endimion75 closed 7 years ago

Endimion75 commented 7 years ago

Set gridding outputs are confusing or don’t match

When using the API path:

/api/data/ccc//image//plate//grid/set

To obtain the gridding I get back:

{grid: [[]], xy1: [[(outer, inner), ...]], xy2: [[(outer, inner), ...]]}

I have successfully used the grid values to draw the centres of the grids.

Now I need to get the coordinates of the borders of the detected colony and as the values do not match. When I use the same set of outter/inner (rows, cols) for grid and xy1/xy2 I target different elements on the plate.

It has become a game of guessing trying to figure what goes where ... any help?

For example, in the image attached, the red circles marks row 0, col, 0 using the gird data. The square also targets row 0, col 0 but using the xy1,xy2.

sc1

Both the grid and the xy1/xy2 have the same outer/inner dimensions. I.e. I can get values for [0,0] to [0,7], but not [0,8] as it is out of index for both. same for rows [11,0] works, but not [12,0].

To me it appears that the plate is inverted for one of them or something like that

local-minimum commented 7 years ago

Just to double check that I understand:

But for that part it to me looks like

We'll look into why the grid and xy1/xy2 don't agree, but please comment on these things I've listed as they may relate.

Endimion75 commented 7 years ago

I think the issue is much clearer now, but there is something funky ... very funky going on ...

Double checking is important:

- There's no confusion or issue with the indexes that they are [row, column]. I follow the specification (outer, inner) and interpret it as (row, column) as you indicated, so if I want to get p1 from xy1 object I would use:

var p1 = scope.PlateGridding.xy1[row][col];

where row goes from 0..11 and col goes from 0..7

- That one of the two, grid or xy1/xy2 have inverted coordinate system compared to the other. This information clarifies most of the issue and makes sense since the xy1/xy2 do start with high pixel values in contrast the grid.

- The cut-out/zoomed in part you are showing doesn't look square though it should be almost perfectly square. Is there something in the css that causes this or are there issues with that the zoomed part is actually not square?

There is no css modifications in the demo I sent you, the rectangle is almost a square if not for a few pixels. To illustrate, if we take the point for row 0, col 0 we get:

p1 = [2337,220] (xy1) p2 = [2549,428] (xy2)

it would make sense that x is the first value and y the second given the name of the containing object but the slice that I get from the API places the plate standing in the short side so x can not be 2548 as it will be drawn outside the image, so I have to take y for the first value and x for the second, in the form yx1, yx2:

x1=220 y1=2337 x2=428 y2=2549

from this we calculate the height and weight:

w = x2-x1 = 208 h= y2-y1 = 212

and draw the rec:

ctx.rect(x1, y1, w, h);

This will draw the rec as the image I sent you originally, one row off

... here is where it gets funky

It happens sometimes than for row 0, col 0 instead of getting:

p1 = [2337,220] (xy1) p2 = [2549,428] (xy2)

I get:

p1 = [2548,222] (xy1)
p2 = [2760,430] (xy2)

These coordinate will draw the square where expected at the bottom row in an inverted coordinate system. The problem is that this is rare. If I run the same image 10 times, it will happen once or less ... I so far have not manage purposely manage to reproduce this behaviour. It just happens to pop here and there.

Comments?

Endimion75 commented 7 years ago

Here is a screen shot when I get the right coordinates without code changes, just running it again and again until it just happens. sc2

local-minimum commented 7 years ago

I'm not really understanding what is happening, but the way you describe it as non-deterministic makes me think it has to do with gridding and the grid falling outside the image and that there are two separate ways to handle this. One for the grid and one for the xy1/xy2. I would not have expected this to be the outcome though. Gridding is done with a random component, so if this is the source of the issue, it isn't strange that refreshing causes it to work sometimes and sometimes not.

Do you have the ability to show the grid when it behaves well and the grid when it behaves badly? Or if this is how things are shown, when things work well, where do the dot and the square end up? Upper left corner?

Endimion75 commented 7 years ago

The grid always works, so the red dot is always where it should be, marking position 0,0

The xy1/xy2 are the ones that are non-deterministic. Most of the time, 0,0 is off by one row, and once every while it gets it right.

If solving this is not trivial I can use the grid center data and "calculate" the square around it. If I do that I would suggest to remove the xy1/xy2 from the gridding response as they are not reliable and can not used.

skymandr commented 7 years ago

Started looking at this today, but would appreciate more detailed description of steps to reproduce, so that I can check this on my computer. Does this problem occur in the master branch, or do I need some features in your development branch to test it?

Endimion75 commented 7 years ago

I got new insight into this matter. The problem is not with the xy1/xy2 objects but with the gridding itself.

You will agree if you carefully observe the two images I uploaded. The gridding if off on one of them!

The problem happens when a gridding is request via:

/api/data/ccc//image//plate//grid/set

And the API returns a success = true, even though the grid is still off.

The question is: is it intended to get a success true when gridding and still get a grid with offset?

I must add that it is weird that I get different girding's even thou I use the same image

skymandr commented 7 years ago

Yes, there appears to be two problems. A pull request which I hope solves the first and easiest (which is not the gridding problem, unfortunately), will be uploaded shortly. The problem this will address is that the indexing starts from the bottom, rather than from the top, as one would expect.

skymandr commented 7 years ago

And there is a random component in the gridding, so it's not that weird that you get different results. The test image is a difficult one to grid, so I'm not surprised to see that it has some problems, but this particular problem mystifies me a bit. If you could test to see if you could reproduce both cases with the new code I would be very happy!

Endimion75 commented 7 years ago

I will, just let me know when the code is ready for me to pull

skymandr commented 7 years ago

I pushed to master directly by mistake. We'll probably have to do something about that, but at the moment you can pull from master to get the updates...

Endimion75 commented 7 years ago

I pulled from master, installed and run the API and tested the CCC as usual.

I got the following error during the gridding phase:


127.0.0.1 - - [29/Jun/2017 17:44:43] "POST /api/calibration/DFSD/image/CalibIm_0/plate/1/grid/set HTTP/1.1" 500 -
Traceback (most recent call last):
  File "/usr/local/lib/python2.7/dist-packages/flask/app.py", line 1997, in __call__
    return self.wsgi_app(environ, start_response)
  File "/usr/local/lib/python2.7/dist-packages/flask/app.py", line 1985, in wsgi_app
    response = self.handle_exception(e)
  File "/usr/local/lib/python2.7/dist-packages/flask_cors/extension.py", line 161, in wrapped_function
    return cors_after_request(app.make_response(f(*args, **kwargs)))
  File "/usr/local/lib/python2.7/dist-packages/flask/app.py", line 1540, in handle_exception
    reraise(exc_type, exc_value, tb)
  File "/usr/local/lib/python2.7/dist-packages/flask/app.py", line 1982, in wsgi_app
    response = self.full_dispatch_request()
  File "/usr/local/lib/python2.7/dist-packages/flask/app.py", line 1614, in full_dispatch_request
    rv = self.handle_user_exception(e)
  File "/usr/local/lib/python2.7/dist-packages/flask_cors/extension.py", line 161, in wrapped_function
    return cors_after_request(app.make_response(f(*args, **kwargs)))
  File "/usr/local/lib/python2.7/dist-packages/flask/app.py", line 1517, in handle_user_exception
    reraise(exc_type, exc_value, tb)
  File "/usr/local/lib/python2.7/dist-packages/flask/app.py", line 1612, in full_dispatch_request
    rv = self.dispatch_request()
  File "/usr/local/lib/python2.7/dist-packages/flask/app.py", line 1598, in dispatch_request
    return self.view_functions[rule.endpoint](**req.view_args)
  File "/home/Luciano/.local/lib/python2.7/site-packages/scanomatic/ui_server/calibration_api.py", line 575, in grid_ccc_image_plate
    grid=grid[:, ::-1, :],
TypeError: list indices must be integers, not tuple
skymandr commented 7 years ago

Sigh, and this is why we use code review! ;) Good error message though, I know exactly where, if not what, the problem is. Guessing that grid is a nested list not an ndarray. Will look into it, and into seeing if we can solve this branching issue.

skymandr commented 7 years ago

There, reset the master branch and made a proper pull request. Please update your code accordingly (currently no changes, but I will commit updates to address the above and any other issues to the new branch instead - I have also assigned you as reviewer(s) so please have a look at the change to see that I haven't done anything atrocious.)

Endimion75 commented 7 years ago

@skymandr, I am not sure if you were referring to me when you request a reviewer, but if you were, I don't know python (like cero) so I don't believe I can help you reviewing, maybe ask @local-minimum?

local-minimum commented 7 years ago

@Endimion75 It would be good if you could review the changes in your system to see if they work as expected in the UI (that the first square and [0, 0] position are the same) and that the image you get when looking at a position is from that position.

Endimion75 commented 7 years ago

I pulled and checkout branch: origin/Issue_133_set_gridding_outputs_confusing I installed the API and ran the CCC with the same image and parameters as always

I got the same error as before I think:

127.0.0.1 - - [30/Jun/2017 13:14:29] "POST /api/calibration/FDFSDF/image/CalibIm_0/plate/1/grid/set HTTP/1.1" 500 - Traceback (most recent call last): File "/usr/local/lib/python2.7/dist-packages/flask/app.py", line 1997, in __call__ return self.wsgi_app(environ, start_response) File "/usr/local/lib/python2.7/dist-packages/flask/app.py", line 1985, in wsgi_app response = self.handle_exception(e) File "/usr/local/lib/python2.7/dist-packages/flask_cors/extension.py", line 161, in wrapped_function return cors_after_request(app.make_response(f(*args, **kwargs))) File "/usr/local/lib/python2.7/dist-packages/flask/app.py", line 1540, in handle_exception reraise(exc_type, exc_value, tb) File "/usr/local/lib/python2.7/dist-packages/flask/app.py", line 1982, in wsgi_app response = self.full_dispatch_request() File "/usr/local/lib/python2.7/dist-packages/flask/app.py", line 1614, in full_dispatch_request rv = self.handle_user_exception(e) File "/usr/local/lib/python2.7/dist-packages/flask_cors/extension.py", line 161, in wrapped_function return cors_after_request(app.make_response(f(*args, **kwargs))) File "/usr/local/lib/python2.7/dist-packages/flask/app.py", line 1517, in handle_user_exception reraise(exc_type, exc_value, tb) File "/usr/local/lib/python2.7/dist-packages/flask/app.py", line 1612, in full_dispatch_request rv = self.dispatch_request() File "/usr/local/lib/python2.7/dist-packages/flask/app.py", line 1598, in dispatch_request return self.view_functions[rule.endpoint](**req.view_args) File "/home/Luciano/.local/lib/python2.7/site-packages/scanomatic/ui_server/calibration_api.py", line 575, in grid_ccc_image_plate grid=grid[:, ::-1, :], TypeError: list indices must be integers, not tuple

I think I can now deploy the latest version of the CCC so that you can test it in real time. Where should I push the changes to this branch or other?

skymandr commented 7 years ago

I have updated the pull request. Please test it with respect to bot Issue #133 and #134 at your convenience. I think it would probably be best if we could move the conversation to be on the pull request instead. (and I think pushing your changes to a different branch might be best, but @local-minimum may have more insight into what is practical(

local-minimum commented 7 years ago

Yes, lets have the discussion on the pull request #135