Skip to content
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

Ssa knee #345

Merged
merged 9 commits into from
Feb 24, 2023
Merged

Ssa knee #345

merged 9 commits into from
Feb 24, 2023

Conversation

alexkjames
Copy link
Contributor

Include knee finding truncation method in ssa. Closes #331

Note: This adds kneed to our dependencies. It looks like a well maintained package, but nonetheless its a new thing that can break.

@alexkjames alexkjames marked this pull request as draft February 23, 2023 00:10
@alexkjames alexkjames marked this pull request as ready for review February 23, 2023 01:02
@alexkjames alexkjames marked this pull request as draft February 23, 2023 01:02
@alexkjames alexkjames marked this pull request as ready for review February 23, 2023 18:55
Copy link
Collaborator

@CommonClimate CommonClimate left a comment

Choose a reason for hiding this comment

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

Looks like a thorough job, but I wonder how that looks in practice. Can you please provide:

  • a notebook example
  • guidance in the docstring about when to use that selection criterion
    ?
    That is, if it seems a lot more objective and performs better than Kaiser or var, let's state that and encourage users to use this as the preferred truncation option. Also, there needs to be a warning about setting online to False if users are going to apply this recursively (or perhaps, warn them not to do that...).

@alexkjames
Copy link
Contributor Author

Re the notebook example, see the attached zip file.

I've also pushed some small changes to the docstring in my most recent commit. Let me know if they seem adequate, happy to elaborate or change them.

ssa_knee_pyleoclim.ipynb.zip

@CommonClimate
Copy link
Collaborator

That's a convincing argument! I would just update the docstring to say that, while no truncation method is imposed by default, if the goal is to enhance the S/N ratio and reconstruct a smooth version of the attractor's skeleton, then the knee-finding method is a good compromise between objectivity and efficiency. Definitely include this in the PyleoTutorial!

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