Skip to content

[Feature] Add new signal quality method for ECG and PPG #1111

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

Conversation

peterhcharlton
Copy link
Contributor

@peterhcharlton peterhcharlton commented Jun 26, 2025

Description

This PR aims at adding the "template matching" method to ecg_quality(). In doing so, I created a couple of generic signal functions.

Proposed Changes

  • I created a generic signal_cyclesegment() function to segment a signal into individual cycles (e.g. to segment an ECG signal into individual heartbeats.
  • I created a generic signal_templatequality() function to perform template matching quality assessment (create an average cycle template, and then compare individual cycles with this template).
  • I updated ppg_quality() to use the new signal_templatequality() function.
  • I updated the ecg_quality() to incorporate template matching ECG quality assessment.

Checklist

Here are some things to check before creating the PR. If you encounter any issues, do let us know :)

  • I have read the CONTRIBUTING file.
  • My PR is targeted at the dev branch (and not towards the master branch).
  • I ran the CODE CHECKS on the files I added or modified and got one error that I don't know how to fix (see comment below).
  • I have added the newly added features to News.rst (if applicable)

Copy link
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

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

Summary of Changes

Hello @peterhcharlton, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request introduces a new, generalized approach to signal quality assessment within the NeuroKit2 library. It adds a generic module for segmenting physiological signals into individual cycles and a module for assessing signal quality based on template matching. This new functionality is then integrated into the existing ECG quality assessment, providing a new 'templatematch' method, and refactors the PPG quality assessment to utilize these new generic functions, centralizing the quality assessment logic.

Highlights

  • New Generic Signal Segmentation: I've added a new signal_cyclesegment() function that can segment any physiological signal (e.g., ECG, PPG) into individual cycles (like heartbeats or pulse waves) based on detected peaks. This function provides a generalized utility for cycle segmentation.
  • New Generic Template-Based Quality Assessment: I've introduced a signal_templatequality() function that provides a generalized way to assess signal quality using methods like 'templatematch' (correlation with an average template) and 'disimilarity' (morphological dissimilarity from a template). This centralizes quality assessment logic.
  • ECG Quality Enhancement: The ecg_quality() function now includes a new 'templatematch' method, allowing users to assess ECG signal quality by comparing individual heartbeats to an average R-peak morphology, based on the Orphanidou et al. (2015) approach.
  • PPG Quality Refactoring: I've refactored the ppg_quality() function to remove its internal, specialized quality assessment logic and instead leverage the new, generic signal_templatequality() function, promoting code reuse and maintainability.
Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.

Feature Command Description
Code Review /gemini review Performs a code review for the current pull request in its current state.
Pull Request Summary /gemini summary Provides a summary of the current pull request in its current state.
Comment @gemini-code-assist Responds in comments when explicitly tagged, both in issue comments and review comments.
Help /gemini help Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist is currently in preview and may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments to provide feedback.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Footnotes

  1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution.

Copy link
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request refactors the template matching quality assessment into a generic signal_templatequality function, used by both ecg_quality and ppg_quality, improving code reuse and maintainability. There are a couple of critical issues in the new helper functions that could lead to data corruption or runtime errors, along with suggestions to improve code clarity and performance.

@peterhcharlton
Copy link
Contributor Author

Apologies, I haven't worked out how to resolve this conflict. The conflict in neurokit2/signal/__init__.py seems to be because I added lines for the new signal_tidypeaksonsets in PR1106, which weren't in my local files when I worked on this new branch. When working on the new branch I also adjusted this file.

@peterhcharlton peterhcharlton marked this pull request as ready for review June 26, 2025 00:38
@peterhcharlton peterhcharlton mentioned this pull request Jun 26, 2025
4 tasks
Copy link
Contributor Author

@peterhcharlton peterhcharlton left a comment

Choose a reason for hiding this comment

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

Apologies - I was a bit out of my depth in trying to resolve the conflicts, and I presume that in doing so I accidentally added some files that shouldn't have been committed. This commit removes those.

@peterhcharlton
Copy link
Contributor Author

I hope this is now ready for review. I haven't understood the failing build test - if it's something I need to work on then I'll be happy to do so. Thanks

@DominiqueMakowski
Copy link
Member

I've enabled the preview of the gemini code assist feature, to see if it helps or not (I heard good things about it 🤷)

Could you comment on the points brought up and let me/gemini know whether it's valid and should be fixed or whether it's wrong and we can ignore?

@peterhcharlton
Copy link
Contributor Author

@DominiqueMakowski , Generally the gemini code comments were helpful. There was:

  • one that was very helpful (i.e. corrected a mistake in my code which presumably would have had implications),
  • one that was probably very helpful (i.e. corrected my mistake, although I don't fully understand what the implications of this mistake would have been),
  • one that was helpful (i.e. made the code neater, but didn't make a difference to its operation), and
  • one that was potentially unhelpful (i.e. would have resulted in changing the algorithm implementation, although admittedly I don't know whether this would have mattered.

I've now made a commit to address the first three of these.

@peterhcharlton
Copy link
Contributor Author

@DominiqueMakowski , Generally the gemini code comments were helpful. There was:

  • one that was very helpful (i.e. corrected a mistake in my code which presumably would have had implications),
  • one that was probably very helpful (i.e. corrected my mistake, although I don't fully understand what the implications of this mistake would have been),
  • one that was helpful (i.e. made the code neater, but didn't make a difference to its operation), and
  • one that was potentially unhelpful (i.e. would have resulted in changing the algorithm implementation, although admittedly I don't know whether this would have mattered.

I've now made a commit to address the first three of these.

I've now addressed the last one too - I had misunderstood the suggested change, but have now made the suggested change.

@peterhcharlton peterhcharlton reopened this Jul 3, 2025
@DominiqueMakowski
Copy link
Member

Out of curiosity, how does the templatematching differs from the existing QRS matching ? From the description it seems fairly similar isn't it?

@peterhcharlton
Copy link
Contributor Author

Out of curiosity, how does the templatematching differs from the existing QRS matching ? From the description it seems fairly similar isn't it?

You're entirely right, it is similar. However, I think there's a key difference:

  • When using the averageQRS method, Lines 151-152 of the old ecg_quality.py assert that within the signal the quality must vary between 0 (lowest) and 1 (highest). Therefore, in a high quality signal, some beats will have low values (e.g. 0), whereas others will have high values (e.g. 1).
  • In contrast, the quality outputted by the templatematching method is determined by the average of the correlations between the template beat morphology and each individual beat's morphology. Therefore, in a high quality signal, all beats can exhibit high values (e.g. >0.95).

Therefore, the templatematching method allows for comparison between signals, whereas the averageQRS method only allows for comparisons within a single signal.

@DominiqueMakowski
Copy link
Member

DominiqueMakowski commented Jul 8, 2025

I see, could we add this clarification in the docstrings? I think it'll be useful for users to get a sense of the differences and domains of applications

@DominiqueMakowski
Copy link
Member

I added a few words

@DominiqueMakowski DominiqueMakowski changed the title Add ecg quality template matching method [Feature] Add new signal quality method for ECG and PPG Jul 8, 2025
@DominiqueMakowski DominiqueMakowski merged commit 63ee012 into neuropsychology:dev Jul 8, 2025
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants