-
Notifications
You must be signed in to change notification settings - Fork 0
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
Adjustable frequencies #44
Conversation
Signed-off-by: Melvin Strobl <[email protected]>
Signed-off-by: Melvin Strobl <[email protected]>
Signed-off-by: Melvin Strobl <[email protected]>
Signed-off-by: Melvin Strobl <[email protected]>
Signed-off-by: Melvin Strobl <[email protected]>
Signed-off-by: Melvin Strobl <[email protected]>
Signed-off-by: Melvin Strobl <[email protected]>
Signed-off-by: Melvin Strobl <[email protected]>
Signed-off-by: Melvin Strobl <[email protected]>
Signed-off-by: Melvin Strobl <[email protected]>
Signed-off-by: Melvin Strobl <[email protected]>
Signed-off-by: Melvin Strobl <[email protected]>
Hello @stroblme! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:
Comment last updated at 2024-11-12 13:11:37 UTC |
ignore, we will tackle that with #45 |
Signed-off-by: Melvin Strobl <[email protected]>
Signed-off-by: Melvin Strobl <[email protected]>
Signed-off-by: Melvin Strobl <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks! Nice realisation of the issue!
This PR (partially) resolves #14 by introducing adjustable number of frequencies.
I did not found a convenient way to adjust all the amplitudes individually, but I think it's good to go with just the # of freqs.
Also we can now adjust the stepsize (because for higher # freqs., learning is tricky..).
Depends on #42