Skip to content

BUG: fix contour plotting for bubblesam detection#33

Merged
tylerjereddy merged 6 commits into
mainfrom
awitmer_fix_contours
Jun 21, 2026
Merged

BUG: fix contour plotting for bubblesam detection#33
tylerjereddy merged 6 commits into
mainfrom
awitmer_fix_contours

Conversation

@adamwitmer

@adamwitmer adamwitmer commented Jun 1, 2026

Copy link
Copy Markdown
Collaborator

Based on previous discussions during manuscript preparation, there is a bug in the main workflow that prevents the contours of segmented areas from being plotted correctly when performing bubble detection using SAM-2. This is because the SAM-2 segmentation maps sometimes contain multiple disjoint areas for a single detected bubble, and the workflow only plots the contour for the smaller of the two areas, resulting in what looks like an empty bounding box for some detected regions. This can be observed by performing detection on the image with "irregular shapes" that is included in the manuscript:

offset 25_left_A6_O_Ph_Raw_541a67b3-783f-485a-9249-e7dc06b309fe-2.tiff

The output of running the workflow using the following incantation and yaml file on main demonstrates the bug:

python run_workflow.py --config test_contour_plotting.yaml --steps detect

test_contour_plotting.yaml
roots:
  work: plot_contours

datasets:
  - id: Test
    method: BubbleSAM
    class: ''
    time_label: ''

    detection:
      img_dir: "offset 25_left_A6_O_Ph_Raw_541a67b3-783f-485a-9249-e7dc06b309fe-2.tiff"
      debug: True
      area_threshold: 1000
      circularity_threshold: 0.0
      model_cfg:
        mask_settings:
          points_per_side: 16
          points_per_batch: 64
          pred_iou_thresh: 0.80
          stability_score_thresh: 0.80
          stability_score_offset: 0.10
          crop_n_layers: 0
          box_nms_thresh: 0.10
          crop_n_points_downscale_factor: 1
          min_mask_region_area: 5
          use_m2m: True
        checkpoint_path: "facebook/sam2.1-hiera-tiny"
        device: "gpu"
Output on main (incorrect contours) image

You can see that within some of the "emtpy" bounding boxes, there are very small blue contours being drawn for those smaller disjoint areas. There is also still a bounding box around the perimeter of the image because the logic used to filter out the segmented background region was using the contour area (which can still be less than max_allowed_area) instead of the bounding box area, which is generally closer to the shape of the image itself.

The output of running the workflow using the same yaml file on this branch shows the correct contours plotted on the input image:

Output on this branch (correct contours) image

The image now contains all the correct contours for each bounding box in the image, and does not contain the bounding box for the background area. There is also a new bounding box/contour for a detected region in the top left corner of the image that was previously filtered out because the function analyze_and_filter_masks was taking only the first rp object from each call of the function sklearn.measure.regionprops, instead of the largest of the multiple regions within a segmentation map.

This PR also allows for the simplification of downstream data structure handling by removing the jagged arrays storing "contour" information in the output dataframe from bubblesam detection and includes changes such as the removal of the fastparquet engine for saving the dataframe as a parquet. This will also simplify the logic used in subsequent PR's for loading the saved dataframes, as noted in i.e. #4 (comment).

AI Disclosure: AI was not used in the original commits to this PR; Explicitly suggested fixes from greptile reviewer were used in subsequent commits.

@greptile-apps

greptile-apps Bot commented Jun 1, 2026

Copy link
Copy Markdown

Reviews (4): Last reviewed commit: "MAINT: add gzip compression back to bubb..." | Re-trigger Greptile

Comment thread neat_ml/bubblesam/bubblesam.py Outdated
Comment thread neat_ml/bubblesam/bubblesam.py Outdated
@tylerjereddy

Copy link
Copy Markdown
Collaborator

I'll make a note to take look here on the same timeline I mentioned for the original PR -- on June 9th.

In the meanwhile, try to make sure that if the LLM reviewer found a problem it is addressed before I circle back.

@tylerjereddy

Copy link
Copy Markdown
Collaborator

Please add an AI usage disclosure to the original comment with sufficient detail please.

@tylerjereddy tylerjereddy added the bug Something isn't working label Jun 11, 2026

@tylerjereddy tylerjereddy left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

The explanation in the OC is a bit hard to follow in sections, but the new image results do look more sensible.

That said, it looks like at least one reasonable bug pointed out during review hasn't been treated with a matching regression test per our usual standard.

Comment thread neat_ml/bubblesam/bubblesam.py
Comment thread neat_ml/bubblesam/bubblesam.py
adamwitmer added a commit that referenced this pull request Jun 15, 2026
* revert changes associated with removal of fastparquet
* add test case for contour filtering to enforce bbox_area calculation
@adamwitmer

adamwitmer commented Jun 15, 2026

Copy link
Copy Markdown
Collaborator Author

Test seem to be failing here because of changes associated with the most recent matplotlib update (3.11.0), and can be reproduced by installing the new version locally (on ARM M2 Macbook Pro) with python -m pip install matplotlib==3.11.0:

FAILED neat_ml/tests/test_bubblesam.py::test_bubblesam_detection_generates_pngs[cpu] - AssertionError: assert 'Error: Image files did not match.\n  RMS Value: 2.640669498266348\n  Expected:  \n    /Users/awitmer/Documents/LANL/2...t-of-awitmer/pytest-3/test_bubblesam_detection_gener0/run/circles_with_mask-failed-diff.png\n  Tolerance: \n    0.0001' is None
FAILED neat_ml/tests/test_bubblesam.py::test_bubblesam_detection_generates_pngs[mps] - AssertionError: assert 'Error: Image files did not match.\n  RMS Value: 2.640669498266348\n  Expected:  \n    /Users/awitmer/Documents/LANL/2...t-of-awitmer/pytest-3/test_bubblesam_detection_gener1/run/circles_with_mask-failed-diff.png\n  Tolerance: \n    0.0001' is None
FAILED neat_ml/tests/test_detection.py::test_visual_regression_debug_overlay - AssertionError: assert 'Error: Image files did not match.\n  RMS Value: 36.22858874840641\n  Expected:  \n    /Users/awitmer/Library/Caches/t...pytest-3/test_visual_regression_debug_o0/overlay/images_Processed_raw_debug-failed-diff.png\n  Tolerance: \n    0.0001' is None
FAILED neat_ml/tests/test_plotting.py::test_visual_regression_on_helpers[titration_diagram-titration_diagram.png-False] - matplotlib.testing.exceptions.ImageComparisonFailure: Image sizes do not match expected size: (3899, 3974, 3) actual size (3903, 3974, 3)
FAILED neat_ml/tests/test_plotting.py::test_visual_regression_on_helpers[plot_phase_diagram-phase_diagram_exp.png-False] - matplotlib.testing.exceptions.ImageComparisonFailure: Image sizes do not match expected size: (4467, 6665, 3) actual size (4468, 6663, 3)
FAILED neat_ml/tests/test_plotting.py::test_visual_regression_on_helpers[plot_phase_diagram-mathematical_model.png-True] - matplotlib.testing.exceptions.ImageComparisonFailure: Image sizes do not match expected size: (4467, 7065, 3) actual size (4468, 7062, 3)
FAILED neat_ml/tests/test_plotting.py::test_plot_two_scatter_visual_regression - matplotlib.testing.exceptions.ImageComparisonFailure: Image sizes do not match expected size: (3894, 3850, 3) actual size (3903, 3855, 3)
FAILED neat_ml/tests/test_plotting.py::test_wrappers_and_pipeline - matplotlib.testing.exceptions.ImageComparisonFailure: Image sizes do not match expected size: (3899, 3974, 3) actual size (3903, 3974, 3)
FAILED neat_ml/tests/test_utils_module.py::test_plotters_visual_and_logic[plot_gmm_decision_regions-gmm_decision_regions.png-region_colors0] - matplotlib.testing.exceptions.ImageComparisonFailure: Image sizes do not match expected size: (764, 803, 3) actual size (764, 805, 3)
FAILED neat_ml/tests/test_utils_module.py::test_plotters_visual_and_logic[plot_gmm_composition_phase-gmm_composition_phase.png-region_colors1] - matplotlib.testing.exceptions.ImageComparisonFailure: Image sizes do not match expected size: (756, 782, 3) actual size (757, 784, 3)

@adamwitmer

Copy link
Copy Markdown
Collaborator Author

Looking at the release docs for matplotlib==3.11.0 (https://matplotlib.org/stable/release/prev_whats_new/whats_new_3.11.0), there are several changes that may be associated with these test failures including with fonts and rcParams.

I also performed a git bisect and identified the first failing commit as follows:

71bb56fde11d15608f869ba655c0aa00311ac5bf is the first bad commit
commit 71bb56fde11d15608f869ba655c0aa00311ac5bf
Author: Albert Y. Shih <ayshih@gmail.com>
Date:   Wed Jun 18 00:10:34 2025 -0400

    Fix bugs in the calculation of the weight array

 extern/agg24-svn/include/agg_image_filters.h | 14 ++++++++++++++
 extern/agg24-svn/src/agg_image_filters.cpp   |  2 ++
 lib/matplotlib/tests/test_image.py           | 19 +++++++++++++------
 3 files changed, 29 insertions(+), 6 deletions(-)

@adamwitmer

Copy link
Copy Markdown
Collaborator Author

@tylerjereddy I tried re-running the failed checks after the most recent merge to main but github actions isn't picking up the new changes automatically (I remember you mentioning this as a way that github actions handles updates to main). Do I need to rebase this branch to include those updates here?

@tylerjereddy

tylerjereddy commented Jun 16, 2026

Copy link
Copy Markdown
Collaborator

You could try rebasing.

I think you need to open another issue as well--I see that NumPy is getting built from source in Python 3.14 CI jobs because your constraints numpy>=1.26.3,<=2.1.3 don't overlap with Python 3.14 support. I believe you should have at least NumPy 2.3.2 to support Python 3.14. Building NumPy from source at a version that doesn't officially support 3.14 is not great/risky. Bumping the NumPy version on 3.14 may also require some other cascading changes to the versions of other deps. I believe the general organization of dependency ranges was brought up in gh-28.

ajwitmer and others added 6 commits June 16, 2026 16:02
* plot largest segmented area from each segmentation map
* fix background filtration by checking bounding box area
* simplify parquet saving
* add test for changes to `analyze_and_filter_masks`
* fix bounding box area calculation
* add axis argument to squeeze operation
* revert changes associated with removal of fastparquet
* add test case for contour filtering to enforce bbox_area calculation
@adamwitmer

Copy link
Copy Markdown
Collaborator Author

I have rebased this branch and force-pushed those changes here and the test suite is now passing. I also opened issue #38 describing the numpy dependency mismatch mentioned in #33 (comment).

@tylerjereddy

Copy link
Copy Markdown
Collaborator

Ok, I poked around again locally and the testing seems sufficiently sensitive to broken source now, so I'll merge, thanks.

@tylerjereddy tylerjereddy merged commit 2c7d02b into main Jun 21, 2026
8 checks passed
@tylerjereddy tylerjereddy deleted the awitmer_fix_contours branch June 21, 2026 18:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants