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

Brightness normalisation #25

Open
wants to merge 15 commits into
base: main
Choose a base branch
from
Open

Conversation

IgorTatarnikov
Copy link
Member

@IgorTatarnikov IgorTatarnikov commented Jan 10, 2025

Before submitting a pull request (PR), please read the contributing guide.

Please fill out as much of this template as you can, but if you have any problems or questions, just leave a comment and we will help out :)

Description

What is this PR

  • Bug fix
  • Addition of a new feature
  • Other

Why is this PR needed?
There can be large intensity differences between each tile. This PR introduces a method to normalise the intensities between each tile.

What does this PR do?

This is done by assuming that the overlapping area should have equal intensity between each tile. This is used to calculate a scaling factor between two tiles. This scaling factor is applied and the comparison is done for the subsequent tiles. For example say an image contains 4 tiles and each tile overlaps with all other tiles:

The first round of comparisons will calculate scaling factors for tile 1 + 2, 1 + 3, 1 + 4. The scaling factors will be applied lazily to the tiles. The second round of comparisons will calculate scaling factors based on tile 2 + 3, 2 + 4.

The final result will be a num_tiles by num_tiles matrix that contains a scaling factor for each tile comparison.

tile_1 tile_2 tile_3 tile_4 final_factor
tile_1 1 1 1 1 1
tile_2 0.8 1 1 1 0.8
tile_3 0.9 0.95 1 1 0.855
tile_4 0.6 0.98 0.96 1 0.565

The final factor is calculated as the product of all scaling factors calculated for that tile. This final scaling factor is then applied to the full resolution image prior to fusing/saving to disk.

How has this PR been tested?

Tests added for new functionality.

Is this a breaking change?

No

Checklist:

  • The code has been tested locally
  • Tests have been added to cover all new functionality (unit & integration)
  • The code has been formatted with pre-commit

Copy link

codecov bot commented Jan 13, 2025

Codecov Report

Attention: Patch coverage is 98.92473% with 1 line in your changes missing coverage. Please review.

Project coverage is 96.95%. Comparing base (7e5f173) to head (87b9b12).

Files with missing lines Patch % Lines
brainglobe_stitch/stitching_widget.py 98.14% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main      #25      +/-   ##
==========================================
+ Coverage   95.54%   96.95%   +1.41%     
==========================================
  Files           6        6              
  Lines         673      756      +83     
==========================================
+ Hits          643      733      +90     
+ Misses         30       23       -7     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@IgorTatarnikov IgorTatarnikov marked this pull request as ready for review January 16, 2025 12:56
Copy link
Member

@lauraporta lauraporta left a comment

Choose a reason for hiding this comment

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

Stitching PR Notes: UI Tests

Screenshot 2025-01-28 at 15 54 04

Problem

Adjusting intensity values during stitching requires a cumbersome process.

Steps Recap

  1. Add tiles to viewer.
  2. Add Fiji path.
  3. Stitch.
  4. Adjust intensity (80p).
    • No feedback when process completes.
  5. Adjust intensity (90p).
    • Feedback: "Intensity already adjusted at this resolution scale."
  6. User deletes all Napari layers, reloads dataset, and restitches (561).
    • Result: Tiles visually unstitched.
  7. Repeat stitching at (647), same result.

Attempted Solution

  1. Delete data folder (suspecting pre-calculated files causing issues).
  2. Re-extract dataset from archive.
  3. Restart Napari.
  4. Repeat process:
    • Add tiles, add Fiji path, stitch → works.

Outcome

Can finally test different intensity values.

)

self.update_tiles_from_mosaic(data_for_napari)

Copy link
Member

Choose a reason for hiding this comment

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

There is no feedback on the end of the adjustment process. Drafting a possible solution:

Suggested change
# there is actually no worker instantiated here, not sure how to do this 🤣
worker.finished.connect(self._on_adjust_intensity_finished)
worker.start()
def _on_adjust_intensity_finished(self):
show_info("Intensity adjustment complete")
# not sure if there should be anything else here

The percentile based on which the normalisation is done.
"""
if self.intensity_adjusted[resolution_level]:
print("Intensity already adjusted at this resolution scale.")
Copy link
Member

Choose a reason for hiding this comment

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

I might want to re-calculate it!

self.stitch_button.setEnabled(False)
self.adjust_intensity_button.setEnabled(False)
worker.finished.connect(self._on_stitch_finished)
worker.start()
Copy link
Member

Choose a reason for hiding this comment

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

Sincerely, I don't understand why it is not re-stitching, there are no filters controlling for saved values 🤔

lauraporta

This comment was marked as duplicate.

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.

2 participants