search

OSIPI / TF2.4_IVIM-MRI_CodeCollection

OSIPI TF2.4: IVIM MRI code collection
Apache License 2.0
9 stars 27 forks source link

Add algorithm comparison and visualization dashboard #67

Closed AhmedBasem20 closed 4 weeks ago

AhmedBasem20 commented 3 months ago

This PR adds an initial visualization dashboard website. I'll set this as a draft for now because I made the deployment for this to replace the documentation workflow. I'm working on creating a sub-page URL for the documentation website to be /documentation and the dashboard page to be on the root URL.

etpeterson commented 3 months ago

Do you have your own branch or repo where we can see it in action?

Is it possible to add a "Loading" message because the initial load takes some time. Or maybe even a progress bar or text if you want to get fancy.

I still see that deselecting columns doesn't remove the true value point.

image

Are other plot types available, like violin?

etpeterson commented 3 months ago

Looking at the range slider, it goes from 2x to 100x, but most of that range isn't used. Maybe it can max out at the maximum useful value?

It might be harder, but a double slider to get a value for the top and bottom of the range would be nice for a few cases - a thought, not a requirement.

It's also a little strange that there are two places where zooming can happen, the "Range" and on the plot "Zoom". It's not immediately clear how they interact and when to use which one. Is it possible to bring those together in some way?

AhmedBasem20 commented 3 months ago

Do you have your own branch or repo where we can see it in action?

Dashboard: https://ahmedbasem20.github.io/TF2.4_IVIM-MRI_CodeCollection Documentation: https://ahmedbasem20.github.io/TF2.4_IVIM-MRI_CodeCollection/documentation

I added a loading state, and will look into a way to remove the ground truth points when deselected.

Are other plot types available, like violin?

Yes, the violin chart is available, there is a diverse number of chart types available here: https://plotly.com/javascript/

etpeterson commented 1 month ago

I noticed some force pushes here. Why is that necessary? Best to avoid if possible.

etpeterson commented 1 month ago

I took the number to be the current value. But it actually seems to be the maximum value. If that's the case there should also be the similar number on the other side.

image
AhmedBasem20 commented 1 month ago

Thanks for the review @etpeterson I made some improvements based on the points you mentioned, please take a look again and let me know what you think.

etpeterson commented 1 month ago

I took the number to be the current value. But it actually seems to be the maximum value. If that's the case there should also be the similar number on the other side. image

Thoughs on this? Edit: Nevermind, I see it's changed to be the current value.

etpeterson commented 1 month ago

@oliverchampion This is looking good to me. Did you have comments or thoughts before merge?

oliverchampion commented 1 month ago

Looks good! Just some remarks: At some settings the boundaries seem to get stuck. E.g.: Knipsel I cannot do anything with the lower boundary. If easy to fix, than yes please, but if challenging, I think the GUI works good enough and one can always zoom using the built in functions :).

AhmedBasem20 commented 1 month ago

Hey Oliver, this bug appears when the range is very small and the full range is already visible. I attempted to debug it but it seems a bit challenging to resolve.

etpeterson commented 1 month ago

@oliverchampion This looks good to me. Feel free to merge if you're satisfied.