Skip to content

export-to-phy as standalone feature, allows compute of PCA features from raw files #161

Open
shrutisss-21 wants to merge 2 commits into
mainfrom
gui
Open

export-to-phy as standalone feature, allows compute of PCA features from raw files #161
shrutisss-21 wants to merge 2 commits into
mainfrom
gui

Conversation

@shrutisss-21

Copy link
Copy Markdown
Collaborator

Added run_export_to_phy, so that you can run reanalyze_bursts and export-to-phy if you didnt run it originally. it will also compute pca features using original h5 file if you deleted your binary file

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

Adds a standalone “export-to-phy” path to the MEA pipeline so users can re-run burst analysis and generate phy_output even if Phy export wasn’t done originally, including an attempt to recompute PCA features by reloading the original .h5 when the binary cache is missing.

Changes:

  • Change generate_reports() defaults and update its export_to_phy call to explicitly disable PC feature computation.
  • Add MEAPipeline.run_export_to_phy() to export a curated unit set to Phy, optionally attaching a recording from binary/ or the original .h5 for PCA computation.
  • In run_mea_pipeline(), when reanalyze_bursts is used, optionally generate phy_output if --export-to-phy was requested and the folder doesn’t already exist.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread IPNAnalysis/mea_analysis_routine.py Outdated
Comment on lines +998 to 1005
si.export_to_phy(
self.analyzer.select_units(clean_units),
output_folder=self.output_dir / "phy_output",
remove_if_exists=True,
copy_binary=False,
compute_pc_features = False # cannot generate pc features without original binary file
)

Copilot AI Mar 26, 2026

Copy link

Choose a reason for hiding this comment

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

This change forces compute_pc_features=False for exports triggered from generate_reports, which disables PCA features even when the binary cache/recording is available (the previous behavior relied on SpikeInterface defaults). If the goal is only to avoid failures when the binary has been deleted, consider determining this flag based on whether a recording is attached/available, or reusing the new run_export_to_phy logic instead of hard-disabling PC features here.

Suggested change
si.export_to_phy(
self.analyzer.select_units(clean_units),
output_folder=self.output_dir / "phy_output",
remove_if_exists=True,
copy_binary=False,
compute_pc_features = False # cannot generate pc features without original binary file
)
# Use the standalone export helper so PC feature computation
# is determined based on recording/binary availability.
self.run_export_to_phy(clean_units)

Copilot uses AI. Check for mistakes.
compute_pc_features = False
if self.recording is not None:
self.logger.info("Using in-memory recording for PCA.")
compute_pc_features = True

Copilot AI Mar 26, 2026

Copy link

Choose a reason for hiding this comment

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

run_export_to_phy sets compute_pc_features=True when self.recording is present, but if self.analyzer was loaded from disk earlier via load_sorting_analyzer(analyzer_folder) it may not have a recording attached (load_recording defaults to False). In that case export_to_phy(..., compute_pc_features=True) can fail or silently skip PC computation. Consider ensuring the analyzer has a recording when compute_pc_features is True (e.g., reload with load_recording=True when binary exists, or call set_temporary_recording(self.recording)).

Suggested change
compute_pc_features = True
try:
# If the analyzer was loaded from disk without a recording, attach the in-memory one.
if hasattr(self.analyzer, "get_recording") and self.analyzer.get_recording() is None:
self.analyzer.set_temporary_recording(self.recording)
compute_pc_features = True
except Exception as e:
self.logger.warning(
"Could not attach in-memory recording for PCA (%s) — will try other sources.",
e,
)

Copilot uses AI. Check for mistakes.
Comment on lines +1092 to +1098
try:
q_metrics = self.analyzer.get_extension("quality_metrics").get_data()
locations = self.analyzer.get_extension("unit_locations").get_data()
q_metrics['loc_x'] = locations[:, 0]
q_metrics['loc_y'] = locations[:, 1]
clean_metrics, _ = self._apply_curation_logic(q_metrics, thresholds)
clean_units = clean_metrics.index.values

Copilot AI Mar 26, 2026

Copy link

Choose a reason for hiding this comment

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

run_export_to_phy always attempts to re-apply curation via _apply_curation_logic and has no way to mirror the existing no_curation option from generate_reports/MEARunOptions. This can unexpectedly export fewer units than the prior run (or than requested). Consider adding a no_curation (or export_all_units) parameter and skipping the curation block when requested, and pass it through from run_mea_pipeline.

Copilot uses AI. Check for mistakes.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
@mandarmp mandarmp self-requested a review March 26, 2026 05:44
@mandarmp

Copy link
Copy Markdown
Collaborator

@shrutisss-21 there are a few copilot suggestions which I need to look into before merging.

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.

3 participants