Skip to content

fix(roisignals): correct neuropil masks and the options output of extractF - #151

Open
ehennestad wants to merge 5 commits into
devfrom
fix-signal-extraction-neuropil-and-options
Open

ehennestad wants to merge 5 commits into
devfrom
fix-signal-extraction-neuropil-and-options

Conversation

@ehennestad

Copy link
Copy Markdown
Collaborator

Two defects in roi signal extraction. I found them by comparing nansen.twophoton.roisignals.extractF with mean pixel values computed by hand on a synthetic stack. The roi signals were exact. The neuropil signals and the returned options were not.

nansen.processing.roi.estimateNeuropilMasks grows a neuropil mask around each roi and ignores the pixels in pixelsToIgnore while it does so. With an image mask it computed pixelsToIgnore = footprintAllRois & ~params.imageMask, which is only true for roi pixels that lie outside the image mask. extractF and nansen.processing.SignalExtractor replace an empty imageMask with true(imageSize), so pixelsToIgnore was false everywhere. The default excludeRoiFromNeuropil = true therefore had no effect: a neuropil mask left out the pixels of its own roi, but held the pixels of every other roi within its reach. On a synthetic stack with a bright roi 3 pixels away from another roi, the mean neuropil signal of that other roi was 3466 instead of 2047. A user-defined imageMask had almost no effect for the same reason. The two conditions are now combined with |.

This changes extracted values. RoiSignals_NeuropilF from the Extract Signals session methods, and neuropil signals saved from the roimanager, include pixels of neighboring rois if they were extracted before this change. dffClassic does not use the neuropil signal and is not affected. dffChenEtAl2013 and dffRoiMinusDffNpil are affected: the activity of a neighboring roi was subtracted from the roi signal as if it were neuropil.

extractF returned P as its second output. P is the struct of default values that the options are parsed against. The parsed options, with roiInd, imageMask, extractFcn and roiMaskFormat resolved, are in params. The Extract Signals session method saves the second output as OptionsSignalExtraction, which therefore always recorded the default values. The second output is now params.

Both functions now declare their options in an arguments block instead of parsing varargin with utility.parsenvpairs. estimateNeuropilMasks takes its five options as name-value arguments, resolves roiInd = "all" itself and throws NANSEN:RoiSignals:ImageMaskSizeMismatch for an image mask of the wrong size. createMasks passes the options by name. extractF takes an optional options struct followed by name-value arguments, because most callers pass a struct, often the one from getDefaultParameters, which also holds the fields that configure the options editor. Options in the struct are passed on as name-value arguments, so they are validated in the same way. Fields that are not options are ignored, and an option given by name still replaces the field of the same name. No option has a default value in the block, because a default could not be told apart from a value given by name and would replace the value in the struct. Default values are added afterwards from getDefaultParameters, which stays the only place where they are defined.

Two things behave differently for callers of extractF. A misspelled option name throws an error instead of being ignored. The struct has to come before the name-value arguments: the extractSignals session method passed it last and is updated, and all other callers in the repository already pass it first. Name=Value syntax now works. utility.parsenvpairs only recognizes option names that are character vectors, so names given as strings were ignored before.

The help text of both functions is rewritten. It documents the size and layout of signalArray, every option with its default value, the second output, the behavior for stacks with more than one current channel, and how a neuropil mask is grown.

nansen.unittest.roisignals.EstimateNeuropilMasksTest and nansen.unittest.roisignals.ExtractFTest are new. They compare the signals with the mean pixel values of hand-made masks and cover the mask options, the calling conventions of extractF and its second output. In testNeuropilSignalLeavesOutNeighboringRois the rois are brighter than the background, so the neuropil signal only equals the background if no neuropil mask holds a pixel of any roi.

The two fixes are the first two commits and change one line each, so they can be picked without the arguments blocks.

Not part of this PR:

  • NansenModules/NANSEN-TwoPhoton holds its own copies of estimateNeuropilMasks and extractF with the same two defects. The draft stack WP0: two-photon migration hygiene (no moves) #129 to WP8: remove the two-photon module from NANSEN and require it from its own repository #137 removes the copies in this repository, so the fixes have to be ported to the module, and the new tests have to move to the twophotontest package when WP2: move the ROI-signals cluster into the two-photon module #132 is rebased.
  • With a scalar roiInd, createMasks only builds masks for the rois that RoI.getNeighboringRoiIndices returns, which are the rois whose center lies within two radii. A roi slightly further away can still lie where the neuropil mask grows, so the neuropil signal of a single-roi extraction can differ from the full extraction. This was hidden while no roi pixels were left out at all.
  • utility.parsenvpairs ignores option names given as strings, so computeDff(F, dffFcn="dffChenEtAl2013") returns the dffClassic result without a warning. computeDff also returns ΔF/F of the first roi only when it is given a samples-by-rois matrix.

🤖 Generated with Claude Code

ehennestad and others added 4 commits September 21, 2026 23:18
estimateNeuropilMasks ignores the pixels in pixelsToIgnore when it
grows a neuropil mask. With an image mask it computed pixelsToIgnore as
footprintAllRois & ~imageMask, which is only true for roi pixels that
lie outside the image mask. extractF and SignalExtractor replace an
empty imageMask with true(imageSize), so pixelsToIgnore was false
everywhere: excludeRoiFromNeuropil had no effect, and the neuropil
signal of a roi included the pixels of neighboring rois. Pixels outside
a user-defined image mask also stayed in the neuropil masks.

Combine the two conditions with |, so that a pixel is left out if it
belongs to a roi or if it lies outside the image mask.

Neuropil signals that were extracted through extractF or
SignalExtractor before this change include pixels of neighboring rois.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
extractF parses its options into params and resolves roiInd, imageMask,
extractFcn and roiMaskFormat in updateParameters. The second output was
P, the struct of default values that the options are parsed against, so
callers always received the defaults. The extractSignals session method
saves this output as OptionsSignalExtraction, which therefore did not
record options that differed from the defaults.

Return params as the second output.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
estimateNeuropilMasks parsed varargin with utility.parsenvpairs against
all signal extraction parameters, although it uses five of them. It now
declares roiInd, imageMask, excludeRoiFromNeuropil,
neuropilExpansionFactor and cellNeuropilSeparation in an arguments
block. An option that is not given takes its default value from
nansen.twophoton.roisignals.extract.getDefaultParameters, which stays
the only place where the default values are defined.

- roiInd="all" is resolved to all rois. It was used as a character
  index before, so the function only worked when the caller had
  resolved it.
- An imageMask whose size differs from the roi masks throws
  NANSEN:RoiSignals:ImageMaskSizeMismatch.
- createMasks passes the five options by name.
- The help text describes how a mask is grown and what each option does.

EstimateNeuropilMasksTest covers the options. Two of its tests fail if
the | in pixelsToIgnore is changed back to &.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
extractF parsed varargin with utility.parsenvpairs, which accepts a
struct, name-value pairs with char names, or both in either order, and
silently ignores names it does not know. Names given as strings, and
therefore all options given with Name=Value syntax, were also ignored.

extractF now takes an optional options struct followed by name-value
arguments that are declared in an arguments block:

- A misspelled option name throws an error.
- Options in the struct are passed on as name-value arguments, so they
  are validated in the same way. Fields that are not options are
  ignored, because callers pass the struct from getDefaultParameters,
  which also holds the fields that configure the options editor.
- An option given by name replaces the field of the same name in the
  struct, as before.
- No option has a default value in the arguments block. Default values
  are added afterwards from getDefaultParameters, because a default
  value in the block could not be told apart from a value given by
  name and would replace the value in the struct.

The struct has to come before the name-value arguments. The
extractSignals session method passed it last and is updated. All other
callers already pass the struct first.

The help text documents the size and layout of signalArray, every
option, the second output and the behavior for stacks with more than
one current channel. The unused variable method is removed.

ExtractFTest compares the signals with the mean pixel values of
hand-made masks and covers the calling conventions and the second
output.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
@github-actions

Copy link
Copy Markdown
Contributor

Test Results

393 tests  +24   393 ✅ +24   40s ⏱️ +13s
 27 suites + 2     0 💤 ± 0 
  1 files   ± 0     0 ❌ ± 0 

Results for commit e93f050. ± Comparison against base commit d8ee8c3.

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.

1 participant