Skip to content

Add Scale-Invariant Heat Kernel Signature classes#125

Merged
gviga merged 4 commits into
mainfrom
scale-hks
May 19, 2026
Merged

Add Scale-Invariant Heat Kernel Signature classes#125
gviga merged 4 commits into
mainfrom
scale-hks

Conversation

@gviga
Copy link
Copy Markdown
Collaborator

@gviga gviga commented Jan 23, 2026

This Pr add a scale invariant version of the Heat Kernel Signature to the possible descriptors to use.

@codecov-commenter
Copy link
Copy Markdown

⚠️ Please install the 'codecov app svg image' to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

❌ Patch coverage is 41.17647% with 10 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
geomfum/descriptor/spectral.py 41.17% 10 Missing ⚠️

📢 Thoughts on this report? Let us know!

@GiLonga
Copy link
Copy Markdown
Collaborator

GiLonga commented Feb 4, 2026

Hello @gviga, I'm starting to review this PR and I already have a question about the two new classes.
The call methods are basically identical, and the init methods too. Would it not be enough to add a method in the base classes that returns the scale-invariant descriptors instead of creating new classes?

I understand the object-oriented approach, but in this case I'm not sure it is trurly useful.

@gviga
Copy link
Copy Markdown
Collaborator Author

gviga commented Feb 5, 2026

Hi @GiLonga ,
Thank tou for reviewing this PR. I'm not sure i got your point:
Are you referring to the fact that both ScaleInvariantHKS and LandMark scale invariant hks have the same call function?
About the object-oriented approach: we need a scale-invariant-hks class with a call function that computes the descriptor, so it is compatible with the API of any descriptor.

@gviga gviga merged commit 2d53517 into main May 19, 2026
3 checks passed
@gviga gviga deleted the scale-hks branch May 19, 2026 10:19
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