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

GUI: Add warning for GUI spectrogram if time too short #937

Open
asoplata opened this issue Nov 14, 2024 · 2 comments · May be fixed by #944
Open

GUI: Add warning for GUI spectrogram if time too short #937

asoplata opened this issue Nov 14, 2024 · 2 comments · May be fixed by #944
Assignees
Labels
hnn-gui HNN GUI
Milestone

Comments

@asoplata
Copy link
Collaborator

It seems that the hotfix I made to improve the spectrogram visuals for the Alpha/Beta tutorial here #928 broke spectrogram display in the GUI when running the default simulation, at least for me. Before that commit it works, but after that commit, if I try to make a Dipole-Spectrogram figure after having run the default simulation that is loaded upon GUI start, I get this:

image

This is almost certainly related to the length of time needed for analysis generation, which is why I needed to lengthen the time for the tests to pass here: c0779ae . How this apparently passed pytest runs, especially test_viz and test_gui, I have no idea.

The fast solution is simple: as we have done recently with the smoothing factor, we introduce a new hotfix to revert the Morlet cycle frequency divisor back to 8. Independent of the default, we should also add a widget so that users can customize the divisor itself.

@asoplata asoplata added bug Something isn't working hnn-gui HNN GUI labels Nov 14, 2024
@asoplata asoplata self-assigned this Nov 14, 2024
@jasmainak
Copy link
Collaborator

I think we have to accept that the GUI is meant for exploration and decide what features make most sense for teaching/exploratory purpose. Then export the data and continue analysis in HNN-core ... we'll soon have combinatorial problems to debug the more features we add to the GUI

@asoplata asoplata changed the title BUG: Previous Morlet cycle change breaks GUI spectrogram using defaults GUI: Add warning for GUI spectrogram if time too short Nov 15, 2024
@asoplata asoplata removed the bug Something isn't working label Nov 15, 2024
@asoplata
Copy link
Collaborator Author

That is a good point that we need to keep GUI additions to the minimum. We discussed this in the Dev meeting on 2024-11-15, and @ntolley and @dylansdaniels confirmed that the above is expected behavior.

In the short term, I will make a PR for this issue that adds a warning to the user. The warning will print to the main log in the event that the spectrogram is not generated due to the length of simulation being too low for the provided frequencies. This is why the spectrogram is not being shown in the default simulation of the GUI - they have clarified that the default GUI simulation is based off of the simulation used in the ERP tutorial, for which it is not useful to calculate a spectrogram.

In the meantime (for Milestone v0.4), @ntolley is going to work on changing plot_tfr_morlet so that it automatically calculates its time x frequency analysis to be appropriate to the activity.

@asoplata asoplata added this to the 0.4 milestone Nov 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
hnn-gui HNN GUI
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants