Skip to content

Fix short trace bug#421

Open
maarzt wants to merge 2 commits into
campaslab:mainfrom
maarzt:fix-short-trace-bug
Open

Fix short trace bug#421
maarzt wants to merge 2 commits into
campaslab:mainfrom
maarzt:fix-short-trace-bug

Conversation

@maarzt
Copy link
Copy Markdown

@maarzt maarzt commented Jul 16, 2025

Description

Running napari-stress in batch on many images often triggered a warning:
Warning: Too few points in trace (length < 5)
And then the surface reconstruction would stop with an index out of bounds exception.
This fix solves the problem. I could compute the surface reconstruction for many
more images.

There is also clearly a bug in the code since the two return statements in estimate_fit_parameters don't match.
This PR solves the problem by replacing the "exceptional" return statement with a "raise ValueError" which is properly
handled in _fancy_edge_fit.

Type of Change

  • Bug fix (non-breaking change which fixes an issue)

How Has This Been Tested?

Worked in my pipeline. But the proper solution would be to increase unit test coverage for this situation trace lenght < 5.

Checklist:

  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • I have commented on my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes Sorry I didn't run the unit tests.

maarzt added 2 commits July 16, 2025 13:43
Currently I don't get a surface reconstruction whenever this warning is
shown:
"Warning: Too few points in trace (length < 5)"
This is because the exceptional return value as the wrong signature
"return [np.nan] * 5, intensity, intensity"
Which leads to an index out of bounds in _fance_edge_fit which is not
handled properly.

This commit replaces the exceptional return value with an ValueError which
is properly handled by _fit_edge_fancy.
@maarzt
Copy link
Copy Markdown
Author

maarzt commented Jul 16, 2025

Dear @jo-mueller,

I hope this PR is helpful. It's a quite small fix. But it helped me getting results for droplet images, when I otherwise didn't get anything.

What I don't understand is why causes the trace length to be to short. Would selecting different reconstruction parameters solve the problem? Here are the reconstruction parameters that I used:

reconstruction_parameters = {
    'n_smoothing_iterations': 15,
    'n_points': 256,
    'n_tracing_iterations': 3,
    'resampling_length': 5,
    'fit_type': 'fancy',   # can be 'fancy' or 'quick'
    'edge_type': 'interior',   # can be 'interior' or 'surface'
    'trace_length': 40,
    'sampling_distance': 2.5,
    'interpolation_method':  'linear',  # can be 'linear' 'cubic' or 'nearest'
    'outlier_tolerance': 1.5,
    'remove_outliers': True,
}

@jo-mueller
Copy link
Copy Markdown
Member

@maarzt Thanks for the PR! One test is failing that checks the results of the surface tracing against the result of an ideal sphere. For me, it'd be ok to simply chance the tolerance of the test a bit to make it pass. In short, the traced points on a surface are converted to spherical coordinates and the radius is compared to the correct radius. Maybe calculating a relative error and setting a more meaningful threshold would be the better way to go, anyway. To adjust, look under _tests/test_reconstruction.py --> test_surface_tracing

@maarzt
Copy link
Copy Markdown
Author

maarzt commented Jul 30, 2025

One test is failing that checks the results of the surface tracing against the result of an ideal sphere.

Hi @jo-mueller, the failing test surprises me. My expectation was that my changes would allow the code to successfully compute results in situations when it previously would have crashed. So it should not change existing results. Something is wrong.

What is the reason for this "Warning: Too few points in trace (length < 5)" message. Understanding this would allow me to construct I unit test. That covers the modified code.

@jo-mueller
Copy link
Copy Markdown
Member

@maarzt I originally put in the warning/error to catch the case when a trace would poke too far out into the space occupied by an image array. And to make sure that the whole process wouldn't fail on this, I decided to fill the array with NaNs in this case to make sure that the whole trace-fitting process wouldn't crash because of a single trace.

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