-
Notifications
You must be signed in to change notification settings - Fork 7
hanningfix #128
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
hanningfix #128
Conversation
behinger
commented
Dec 13, 2024
- FormulaOnsets
- initial sequence tryout
- fix bug in predef_eeg
- fix \beta missing
- forgot the end
- half way through to success for sequence designs or something
- everythinig except sequencelength seems to be working now
- added string sequence tests
- small doc update
- added jitter to the '_' trial divisor
- generalized LinearModelComponent to arbitrary functions instead of vectors
- bugfix with endless loop due to multiple dispatch
- function component for multi-subject
- forgot to define offset in LinearModelFunction
- Improve documentation especially quickstart, home and power analysis
- adapted the order of reference overviews and adapted titles
- Updated quickstart page
- minor changes
- fixed docstrings for predef_eeg and predef_2x2
- added draft of design types reference page
- Update quickstart.jl
- Add UnfoldSim logo to the documentation
- Finished experimental design reference page
- Replace logo file
- Update logo file
- Delete docs/src/assets/logo.svg
- Add logo as png file
- Added intro paragraph for Simulate ERP tutorial
- Improved docstrings for single- and multi-subject design
- Fixed simulate docstring
- Added cross references in docstrings
- Added intro sentences, matched titles and sidebar, reordered pages and added collapsible setup blocks
- Update noise.jl
- Update src/noise.jl
- Update src/noise.jl
- Update src/noise.jl
- add empty line for formatting reasons
- Update docs/literate/reference/noisetypes.jl
- Update docs/literate/reference/overview.jl
- Update docs/literate/reference/overview.jl
- Update docs/literate/reference/noisetypes.jl
- added docstring
- renamed to have the formula at the end
- merge fix, double definition of function
- component function test + docstring
- better docs
- added unittests
- fixed small wording
- fix tutorial with v0.3 renaming of simulate to simulate_component
- fix newComponent tutorial
- fix sequence design rng
- fix sequence test
- fix rng docs
- fix Change Simulation struct field "components" from Type Dict to NamedTuple Type. #124, explicitly cast the dict
- not sure where this error came in...
- hanning upgrade
- checking stuff
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.
I think once we discuss the points I raised, this PR is good to go! Thanks @behinger!
| function DSP.hanning(duration, offset, sfreq) | ||
| signal = hanning(Int(round(duration * sfreq))) | ||
| return pad_array(signal, -Int(round(offset * sfreq / 2)), 0) | ||
| function DSP.hanning(width, offset, sfreq) |
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.
I wondered whether we should allow that width can be 0, because then the DSP.hanning function returns an empty vector, which is then zero-padded by the UnfoldSim hanning function.
However, there may be a case that I'm not thinking of right now where this could be desirable.
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.
ah sorry, missed that one. If it doesnt error, it seems fine for me; maybe you want to parametrically change from 0:10 the width and assume 0 is no signal?
Co-authored-by: Judith Schepers <judith.schepers@vis.uni-stuttgart.de>
Co-authored-by: Benedikt Ehinger <benedikt.ehinger@vis.uni-stuttgart.de>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
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.
I needed to fix the test (since after including them, they failed) for the new implementation with (signal+1)/2.
Now everything should work and I'll merge once CI run through.