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

790 add scientific use case frb to dashboard #832

Merged
merged 44 commits into from
Aug 13, 2024

Conversation

laurasootes
Copy link
Contributor

@laurasootes laurasootes commented Jul 31, 2024

This PR adds the FRB use case to the dashboard.

The FRB case and data is quite different from the weather example, and therefore requires a lot of FRB-specific data conversions and such, making the code not that pretty. It only works for RISE (as is the case in the notebook). Since it is not straightforward, I made an issue to get it to also work for LIME #836 .
Fixes #790

Additionally, I split the left menu; it now starts with a choice for example or own data, and only then shows the choices/inputs required depending on example/own data. I think this makes the dashboard less confising and less visually overwhelming: you do not see choices and buttons you do not need. Extension to #818

I updated the tests in accordance with the new input data layout

Also, added the DIANNA.utils.downloader for the data used in the dashboard. Fixes #839

@laurasootes laurasootes linked an issue Jul 31, 2024 that may be closed by this pull request
@laurasootes laurasootes marked this pull request as draft July 31, 2024 19:30
@laurasootes laurasootes marked this pull request as ready for review August 7, 2024 10:11
Copy link
Member

@SarahAlidoost SarahAlidoost left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@laurasootes nice work! 👍 I like the new interface with separate sections in the left menu, much cleaner now. I was able to run the dashboard, and its tests. Just a few comments on the changed file. Also, I noticed that the quality of frb images is not the same as those in the notebook. can you check it? see here:

from dashboard
frb_dashboard

from notebook
frb_nb

@laurasootes
Copy link
Contributor Author

@SarahAlidoost Thanks for your review!
I have implemented your suggestions:

  • for the FRB data, I used the downloader function now, and also implemented these for the other examples
  • for the naming of the variables related to the FRB adjustments; I renamed them and added some inline comments as you suggested. I hope the code is clearer now

As for the result; the difference is caused by the parameters used. Changing the model parameters gives me this:

image

(The difference in darkness is caused by the overlay, which was not done in the notebook)

@SarahAlidoost
Copy link
Member

SarahAlidoost commented Aug 13, 2024

As for the result; the difference is caused by the parameters used. Changing the model parameters gives me this:

@laurasootes I amnot sure if the difference is caused by the parameters. Here is the screenshot of my dashboard with the same parameters as yours. There are two main differences:

  1. the figure's aspect ratio: yours is square, while mine is rectangular. I checked the explanation[0, :, ::-1].T.shape that is (32, 64).
  2. the x and y axis labels are missing in my figure. To check this, I saved the fig returned by plot_image() as fig.savefig('...') or in the notebook after 4 - Plot the explanation and input data next to each other visualization.plot_image(heatmap) returns the same plot as mine. Can you please check these two issues?

frb_dashboard

@laurasootes
Copy link
Contributor Author

As for the result; the difference is caused by the parameters used. Changing the model parameters gives me this:

@laurasootes I amnot sure if the difference is caused by the parameters. Here is the screenshot of my dashboard with the same parameters as yours. There are two main differences:

  1. the figure's aspect ratio: yours is square, while mine is rectangular. I checked the explanation[0, :, ::-1].T.shape that is (32, 64).
  2. the x and y axis labels are missing in my figure. To check this, I saved the fig returned by plot_image() as fig.savefig('...'). Can you please check these two issues?

frb_dashboard

To compare with the one in the notebook I temporarily fixed the aspect radio and added the axis labels in the dashboard (and removed the background original image), so that I could check how far the red part of the explanation reaches as the ’shape’ of the explanation was my concern. (I did not add the axis labels in the dashboard as I think it does not add any information for the explanation). Besides these formatting things I believe your result is the same as mine right?

@SarahAlidoost
Copy link
Member

As for the result; the difference is caused by the parameters used. Changing the model parameters gives me this:

@laurasootes I amnot sure if the difference is caused by the parameters. Here is the screenshot of my dashboard with the same parameters as yours. There are two main differences:

  1. the figure's aspect ratio: yours is square, while mine is rectangular. I checked the explanation[0, :, ::-1].T.shape that is (32, 64).
  2. the x and y axis labels are missing in my figure. To check this, I saved the fig returned by plot_image() as fig.savefig('...'). Can you please check these two issues?

frb_dashboard

To compare with the one in the notebook I temporarily fixed the aspect radio and added the axis labels in the dashboard (and removed the background original image), so that I could check how far the red part of the explanation reaches as the ’shape’ of the explanation was my concern. (I did not add the axis labels in the dashboard as I think it does not add any information for the explanation). Besides these formatting things I believe your result is the same as mine right?

thanks for the explanations 👍 , now I understand what causes the differences. Yes, if I add the aspect ratio and the lables, the figures are the same. 😄

@laurasootes
Copy link
Contributor Author

@SarahAlidoost thanks for your suggestions. I removed the background from the plot, which also removed the normalisation problem :)

Copy link
Member

@SarahAlidoost SarahAlidoost left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@laurasootes thanks for addressing the comments 👍

@laurasootes laurasootes merged commit 93c624e into main Aug 13, 2024
5 of 16 checks passed
@laurasootes laurasootes deleted the 790-add-scientific-use-case-frb-to-dashboard branch August 23, 2024 06:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

use new download functionality in dashboard Add scientific use case (FRB) to dashboard
2 participants