jonescompneurolab / hnn-core

Simulation and optimization of neural circuits for MEG/EEG source estimates
https://jonescompneurolab.github.io/hnn-core/
BSD 3-Clause "New" or "Revised" License
55 stars 52 forks source link

[MRG] Add RMSE #636

Closed chenghuzi closed 1 year ago

chenghuzi commented 1 year ago

As titled.

Demo (updated): image

chenghuzi commented 1 year ago

@jasmainak maybe we can finish this first and turn that histogram stuff into another PR?

codecov-commenter commented 1 year ago

Codecov Report

Merging #636 (a6902c4) into master (cdd4050) will decrease coverage by 0.31%. The diff coverage is 64.86%.

:exclamation: Current head a6902c4 differs from pull request most recent head d20424a. Consider uploading reports for the commit d20424a to get more accurate results

:mega: This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more

@@            Coverage Diff             @@
##           master     #636      +/-   ##
==========================================
- Coverage   92.19%   91.89%   -0.31%     
==========================================
  Files          22       22              
  Lines        4282     4316      +34     
==========================================
+ Hits         3948     3966      +18     
- Misses        334      350      +16     
Impacted Files Coverage Δ
hnn_core/gui/_viz_manager.py 86.93% <64.86%> (-2.91%) :arrow_down:

... and 1 file with indirect coverage changes

chenghuzi commented 1 year ago

We have discussed adding the feature of a layered histogram into the default figure. This will be a separate PR. For now, maybe it would be better for us to merge this one first.

jasmainak commented 1 year ago

I just gave this a shot but I'm confused how exactly to use it:

image

Target data: None means what? I can't figure out what to compare to what. Can you share what you see and/or what you did?

jasmainak commented 1 year ago

Also, some of the loaded data starts to appear in the simulations ... that's a bit confusing:

image

I think you should maintain separate lists of simulations and loaded data

chenghuzi commented 1 year ago

I just gave this a shot but I'm confused how exactly to use it:

image

Target data: None means what? I can't figure out what to compare to what. Can you share what you see and/or what you did?

None means there's no comparison in the current plot. It's not every time that the user want to compare some simulations against others. Without this null choice, the system will use the first value in options to compare.

chenghuzi commented 1 year ago

Also, some of the loaded data starts to appear in the simulations ... that's a bit confusing:

image

I think you should maintain separate lists of simulations and loaded data

I don't think so, we discussed this before and agreed on the current solution, i.e., to put everything under the same structure as essentially they are the same except that there's no Net objects in recorded data. But I renamed a bit in the latest commit to make it more clear to the user.

rythorpe commented 1 year ago

Is this ready to go or were there more changes to be made after yesterday's mtg?

ntolley commented 1 year ago

@chenghuzi happy to merge whenever you give the go ahead

chenghuzi commented 1 year ago

@chenghuzi happy to merge whenever you give the go ahead

LGTM

jasmainak commented 1 year ago

sorry this was an open tab on my browser ... just realized it has been merged, but feel free to clarify my question :)