UBC-MDS / simpl_eeg_capstone

MDS Capstone Project
https://ubc-mds.github.io/simpl_eeg_capstone/
MIT License
2 stars 1 forks source link

Final report draft - Feedback #304

Closed joelostblom closed 3 years ago

joelostblom commented 3 years ago

@MattTPin @sbabicki @mgaroub @YikiSu

The repo and final data product is looking to be in great shape! The report is good overall, but I think there is room to flesh out the motivation and pros & cons more on several places in the report. You have done this nicely when explaining your choice of streamlit as the dashboarding library but it is missing elsewhere e.g. for each of the individual visualizations (what value to each of these bring to the researcher? why do we need a brain map and a 3D skull map, why is 2D important, why are there 2 connectivity circles (talk about the new viz you created that is not available anywhere else)).  This shouldn't be an issue length-wise as you are currently around 1300/2000 words. Below are specific comments I made on different section of the report (not as nicely formatted as last time in the interest of time....):

“Executive Summary” (“Final_Report.pdf”, p. 1) Make this a summary of the entire report (including your results), rather than just your plan (it's a typo that is say "proposal" here in the instructions). Move the enumeration of the goals to the end of the introduction and elaborate on the importance/value of each one in 1-2 sentences (some of this is already in the end of the intro so it will tie in nicely).

“3.2 Introduction” (“Final_Report.pdf”, p. 1) Clear, well-written intro

“Figure 1: Examp” (“Final_Report.pdf”, p. 1) Too small, I can't read any labels in the figure.

“Data Science Methods” (“Final_Report.pdf”, p. 2) Also include brief description of the data and any preprocessing you did.

“Clear documentation” (“Final_Report.pdf”, p. 2) GREAT documentation! Thorough guide and great idea with the demo example animations for the user interface (if it is an easy fix I would make them a bit bigger, it is hard to read the numbers and see what you are clicking). It's also nice that you show how they can use any MNE functions with the epochs in case they need to extend it in the future. Also great that you have gif and animated versions for the functions. A few minor comments:

“the code is tested” (“Final_Report.pdf”, p. 2) Comprehensive tests, well done!

“Figure 2: Visualization comparison” (“Final_Report.pdf”, p. 2) This figure is great and I think part of the motivation I am asking for can be addressed by writing out some of the key pros & cons indicated in this figure in the appropriate places of the report.

“In this context, “brain states” refers to a distinct temporal pattern of electrical activity within the 19 channel EEG data.” (“Final_Report.pdf”, p. 2) Start with this general description of the problem as well as a high level indication of what the data looks like that you want to cluster.

“3.4.1 Python Package” (“Final_Report.pdf”, p. 2) Include motivation/justification to why you have created these plots. What purpose/value to they add to the partner? The same way you added this to your slides after we discussed it.

“3.4.2 User Interface” (“Final_Report.pdf”, p. 5) And you can download figures, that's pretty important for them. Also describe why anyone would even use the package instead of the UI: e.g. more control/functionality working in notebooks, etc (maybe goes best as an intro to the pkg section).

“2 Stretch goal” (“Final_Report.pdf”, p. 5) This is currently not documented or delivered in a way that makes it easy for the partner to understand and extend your work.

“particularly with the SOM method which we deem to be the most promising” (“Final_Report.pdf”, p. 6) Why?

sbabicki commented 3 years ago

Thanks Joel! We have addressed almost all of the feedback in the latest version, with the exception of:

A checklist of tasks that were addressed based on your feedback can be found in the following issues: https://github.com/UBC-MDS/simpl_eeg_capstone/issues/207 https://github.com/UBC-MDS/simpl_eeg_capstone/issues/113