SpeciesFileGroup / taxonworks

Workbench for biodiversity informatics.
http://taxonworks.org
Other
86 stars 25 forks source link

[Bug]: "TODO" images failing to calculate for unknown reason #3070

Closed tmcelrath closed 1 year ago

tmcelrath commented 2 years ago

Steps to reproduce the bug

1. When I upload an image like this to SQED TODO:
https://sfg.taxonworks.org/tasks/accessions/breakdown/sqed_depiction/138106

https://sfg.taxonworks.org/system/images/image_files/000/822/905/original/img_0129.jpg?1659975339
2. Then click recalculate.
3. It gives a "404"/backend error. 
...

Screenshot

img_0129

Expected behavior

Should calculate

Additional Screenshots

No response

Environment

Production

Sandbox Used

No response

Version

0.28.0

Browser Used

Chrome

tmcelrath commented 2 years ago

This is blocking progress on a grant-funded project, so I need an answer quickly.

LocoDelAssembly commented 2 years ago

With the changes below in sqed gem (which are just an experiment) I made it stop crashing, but the results are no good: image

What pattern have you selected for this image @tmcelrath? From the options I couldn't tell which is the correct one so I picked seven segments (but layout doesn't seem to really match)

diff --git a/lib/sqed/boundary_finder/color_line_finder.rb b/lib/sqed/boundary_finder/color_line_finder.rb
index c2bcdf7..97eb1de 100644
--- a/lib/sqed/boundary_finder/color_line_finder.rb
+++ b/lib/sqed/boundary_finder/color_line_finder.rb
@@ -64,8 +64,8 @@ class Sqed::BoundaryFinder::ColorLineFinder < Sqed::BoundaryFinder
       boundaries.set(3, [0, horizontal.y_for(1), bottom.width_for(0), bottom.height_for(0) ])

     when :lep_stage
-      top_bottom_split = Sqed::BoundaryFinder.color_boundary_finder(image: image, scan: :columns, boundary_color: boundary_color)              # detect vertical division [array]
-      left_right_split = Sqed::BoundaryFinder.color_boundary_finder(image: image, sample_subdivision_size: 2, boundary_color: boundary_color)  # detect horizontal division [array]
+      top_bottom_split = Sqed::BoundaryFinder.color_boundary_finder(image: image, scan: :columns, boundary_color: boundary_color) || return              # detect vertical division [array]
+      left_right_split = Sqed::BoundaryFinder.color_boundary_finder(image: image, sample_subdivision_size: 2, boundary_color: boundary_color) || return # detect horizontal division [array]

       boundaries.set(6, [0, top_bottom_split[2], left_right_split[0], image.rows - top_bottom_split[2]] )

@@ -73,7 +73,7 @@ class Sqed::BoundaryFinder::ColorLineFinder < Sqed::BoundaryFinder

       left_top_split =
         SqedUtils.corrected_frequency(
-          Sqed::BoundaryFinder.color_boundary_finder(image: left_top_image, boundary_color: boundary_color),
+          Sqed::BoundaryFinder.color_boundary_finder(image: left_top_image, boundary_color: boundary_color) || return,
           max_width: left_top_image.columns,
           width_factor: 1.8
       )
@@ -87,7 +87,7 @@ class Sqed::BoundaryFinder::ColorLineFinder < Sqed::BoundaryFinder

       bottom_right_split =
         SqedUtils.corrected_frequency(
-          Sqed::BoundaryFinder.color_boundary_finder(image: bottom_right_image, boundary_color: boundary_color, scan: :rows),
+          Sqed::BoundaryFinder.color_boundary_finder(image: bottom_right_image, boundary_color: boundary_color, scan: :rows) || return,
           max_width: bottom_right_image.columns,
           width_factor: 1.8
       )
@@ -98,7 +98,7 @@ class Sqed::BoundaryFinder::ColorLineFinder < Sqed::BoundaryFinder

       bottom_right_left_split =
         SqedUtils.corrected_frequency(
-          Sqed::BoundaryFinder.color_boundary_finder(image: bottom_right_left_image, scan: :columns, boundary_color: boundary_color),
+          Sqed::BoundaryFinder.color_boundary_finder(image: bottom_right_left_image, scan: :columns, boundary_color: boundary_color) || return,
           max_width: bottom_right_left_image.columns,
           width_factor: 1.8
       )
@@ -113,8 +113,8 @@ class Sqed::BoundaryFinder::ColorLineFinder < Sqed::BoundaryFinder
       ])

     when :lep_stage2
-      top_bottom_split = Sqed::BoundaryFinder.color_boundary_finder(image: image, scan: :columns, boundary_color: boundary_color)              # detect vertical division [array]
-      left_right_split = Sqed::BoundaryFinder.color_boundary_finder(image: image, sample_subdivision_size: 2, boundary_color: boundary_color)  # detect horizontal division [array]
+      top_bottom_split = Sqed::BoundaryFinder.color_boundary_finder(image: image, scan: :columns, boundary_color: boundary_color) || return              # detect vertical division [array]
+      left_right_split = Sqed::BoundaryFinder.color_boundary_finder(image: image, sample_subdivision_size: 2, boundary_color: boundary_color) || return  # detect horizontal division [array]

       boundaries.set(6, [0, top_bottom_split[2], left_right_split[0], image.rows - top_bottom_split[2]] ) # OK

@@ -122,7 +122,7 @@ class Sqed::BoundaryFinder::ColorLineFinder < Sqed::BoundaryFinder

       left_top_split =
         SqedUtils.corrected_frequency(
-          Sqed::BoundaryFinder.color_boundary_finder(image: left_top_image, boundary_color: boundary_color),
+          Sqed::BoundaryFinder.color_boundary_finder(image: left_top_image, boundary_color: boundary_color) || return,
           max_width: left_top_image.columns,
           width_factor: 1.8
       )
@@ -136,7 +136,7 @@ class Sqed::BoundaryFinder::ColorLineFinder < Sqed::BoundaryFinder

       bottom_right_split =
         SqedUtils.corrected_frequency(
-          Sqed::BoundaryFinder.color_boundary_finder(image: bottom_right_image, boundary_color: boundary_color, scan: :rows),
+          Sqed::BoundaryFinder.color_boundary_finder(image: bottom_right_image, boundary_color: boundary_color, scan: :rows) || return,
           max_width: bottom_right_image.columns,
           width_factor: 1.8
       )
@@ -151,7 +151,7 @@ class Sqed::BoundaryFinder::ColorLineFinder < Sqed::BoundaryFinder

       bottom_right_left_top_bottom_split =
         SqedUtils.corrected_frequency(
-          Sqed::BoundaryFinder.color_boundary_finder(image: bottom_right_left_image, scan: :columns, boundary_color: boundary_color),
+          Sqed::BoundaryFinder.color_boundary_finder(image: bottom_right_left_image, scan: :columns, boundary_color: boundary_color) || return,
           max_width: bottom_right_left_image.columns,
           width_factor: 1.8
       )
@@ -202,8 +202,8 @@ class Sqed::BoundaryFinder::ColorLineFinder < Sqed::BoundaryFinder
       boundaries.set(2, [ vertical.x_for(1), right.y_for(1), right.width_for(1), right.height_for(1)] )

     when :seven_slot
-      top_bottom_split = Sqed::BoundaryFinder.color_boundary_finder(image: image, scan: :columns, boundary_color: boundary_color)              # detect vertical division [array]
-      left_right_split = Sqed::BoundaryFinder.color_boundary_finder(image: image, sample_subdivision_size: 1, boundary_color: boundary_color)  # detect horizontal division [array]
+      top_bottom_split = Sqed::BoundaryFinder.color_boundary_finder(image: image, scan: :columns, boundary_color: boundary_color) || return              # detect vertical division [array]
+      left_right_split = Sqed::BoundaryFinder.color_boundary_finder(image: image, sample_subdivision_size: 1, boundary_color: boundary_color) || return  # detect horizontal division [array]

       boundaries.set(0, [0, 0, left_right_split[0], top_bottom_split[0]])
       boundaries.set(6, [0, top_bottom_split[2], left_right_split[0], image.rows - top_bottom_split[2]] )
@@ -213,7 +213,7 @@ class Sqed::BoundaryFinder::ColorLineFinder < Sqed::BoundaryFinder

       right_top_split = ::SqedUtils.corrected_frequency(
         Sqed::BoundaryFinder.color_boundary_finder(
-          image: right_top_image, boundary_color: boundary_color, scan: :rows),
+          image: right_top_image, boundary_color: boundary_color, scan: :rows) || return,
           width_factor: 1.8,
           max_width: right_top_image.columns
       ) # vertical line b/w 1 & 2, use "corrected_frequency" to account for color bleed from previous crop
@@ -225,7 +225,7 @@ class Sqed::BoundaryFinder::ColorLineFinder < Sqed::BoundaryFinder
         Sqed::BoundaryFinder.color_boundary_finder(
           image: right_bottom_image,
           scan: :columns,
-          sample_subdivision_size: 2, boundary_color: boundary_color),
+          sample_subdivision_size: 2, boundary_color: boundary_color) || return,
           width_factor: 1.8,
           max_width: right_bottom_image.rows) # horizontal line b/w (5,3) & 4, use "corrected_frequency" to account for color bleed from previous crop

@@ -251,13 +251,13 @@ class Sqed::BoundaryFinder::ColorLineFinder < Sqed::BoundaryFinder
       boundaries.set(3, [0, left.y_for(1), left.width_for(1), left.height_for(1) ])

     when :vertical_split
-      t = Sqed::BoundaryFinder.color_boundary_finder(image: image, boundary_color: boundary_color)  #detect vertical division
+      t = Sqed::BoundaryFinder.color_boundary_finder(image: image, boundary_color: boundary_color) || return  #detect vertical division
       return if t.nil?
       boundaries.set(0, [0, 0, t[0], image.rows])  # left section of image
       boundaries.set(1, [t[2], 0, image.columns - t[2], image.rows])  # right section of image

     when :horizontal_split
-      t = Sqed::BoundaryFinder.color_boundary_finder(image: image, scan: :columns, boundary_color: boundary_color)  # set to detect horizontal division
+      t = Sqed::BoundaryFinder.color_boundary_finder(image: image, scan: :columns, boundary_color: boundary_color) || return  # set to detect horizontal division
       return if t.nil?

       boundaries.set(0, [0, 0, image.columns, t[0]])  # upper section of image
diff --git a/lib/sqed/extractor.rb b/lib/sqed/extractor.rb
index f26e9a2..b6d969d 100644
--- a/lib/sqed/extractor.rb
+++ b/lib/sqed/extractor.rb
@@ -34,7 +34,7 @@ class Sqed
       r.sections = metadata_map.keys.sort.collect{|k| metadata_map[k]}

       # assign the images to the result
-      boundaries.each do |section_index, coords|
+      boundaries.reject { |i, c| c.include?(nil) }.each do |section_index, coords|
         section_type = metadata_map[section_index]

         r.send("#{section_type}_image=", extract_image(coords))
@@ -47,7 +47,7 @@ class Sqed
         if parsers = SqedConfig::SECTION_PARSERS[section_type]
           section_image = r.send("#{section_type}_image")
           updated = r.send(section_type)
-
+          next if section_image.nil?
           parsers.each do |p|
             parsed_result = p.new(section_image).get_text(section_type: section_type)
             updated[p::TYPE] = parsed_result if parsed_result && parsed_result.length > 0
mjy commented 2 years ago

Has the image been asigned the correct stage type?

tmcelrath commented 2 years ago

Yes, this is either "LepStage" or "LepStage2". Have you tried it with that stage type? That was the first thing I checked.

LocoDelAssembly commented 2 years ago

image

LocoDelAssembly commented 2 years ago

image After re-saving with GIMP and answering yes if I wanted to rotate the image to not rely on EXIF rotation metadata.

tmcelrath commented 2 years ago

Ah, right, the rotation stuff! Let me go check it that works. I forgot to ask if he was doing that or not.

LocoDelAssembly commented 2 years ago

It is rotated 180 degrees.

mjy commented 2 years ago

When we catch that there is an error, and we show we can't compute boundaries, then we shoudl show a computed image to show the curator that it is a rotation problem?

tmcelrath commented 2 years ago

Yes, that would help immensely.

LocoDelAssembly commented 2 years ago

sqed should honor EXIF rotation I think. Not sure why it isn't doing that already.

LocoDelAssembly commented 2 years ago

Just noticed that thumbails also appear upside down so a TW problem as well. By changing last like of crop_image in sqed with @stage_image = @stage_image.auto_orient I get this result:

image

So it looks that GIMP did more when I saved the image rotated to correct orientation, and in fact you can probably notice the red hue is a bit different in the original image with respect to the GIMP-generated one.

mjy commented 2 years ago

Does sqed auto-orient during processing of borders?

LocoDelAssembly commented 2 years ago

It doesn't, above result is after I made that small change, apparently you have to explicitly tell RMagick to do that. Furthermore in TW auto-orient is explicitly disabled at https://github.com/SpeciesFileGroup/taxonworks/blob/fb524073424054309599c48220a8bafda03f45e7/lib/vendor/paperclip/rotator.rb#L4 . It is fine to keep original files as-is, but I think derivatives ones that are meant to be displayed in UI should be auto-rotated since browsers almost never honor EXIF rotation (and perhaps EXIF data is even stripped out in derivatives?).

mjy commented 2 years ago

Completely agree.

mjy commented 1 year ago

I have updated to

Screenshot 2023-09-11 at 2 16 13 PM