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

fix: metric flag while visualizing #1148

Open
wants to merge 6 commits into
base: master
Choose a base branch
from
Open

fix: metric flag while visualizing #1148

wants to merge 6 commits into from

Conversation

kanishk16
Copy link
Contributor

@kanishk16 kanishk16 commented Jun 6, 2022

Checklist

GitHub

  • I've given this PR a concise, self-descriptive, and meaningful title
  • I've linked relevant issues in the PR body
  • I've applied the relevant labels to this PR
  • I've assigned a reviewer

PR contents

Description

This PR resolves the bug while using the --metric flag in the visualize_and_compare_testing_models.py script. Furthermore, the datapoints within each violinplot are made fixed to be interactive as they earlier usesd to be. Although, the visualization works completely fine but fails in case of using a Jupyter Notebook or a Google Colab Notebook.

To test this PR:

  1. Download & extract the zip file: viz_test.zip

  2. run ivadomed_visualize_and_compare_testing_models as suggested in the docs without the metadata flag.
    To test the --metric flag, select any evaluation metric from evaluation_3Dmetrics.csv including from dice_class0. Also, the features in the docs should be enabled.

Linked issues

resolves #1138

@kanishk16 kanishk16 self-assigned this Jun 6, 2022
@kanishk16 kanishk16 added the bug category: fixes an error in the code label Jun 6, 2022
Copy link
Member

@mariehbourget mariehbourget left a comment

Choose a reason for hiding this comment

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

Thank you @kanishk16 for looking into this!
The metric argument parsing is working well now.

Unfortunately, I'm unable to review the visualization, as it is also not working on my setup (WSL) with the following error:

UserWarning: Matplotlib is currently using agg, which is a non-GUI backend, so cannot show the figure.
  plt.show(block=True)

I have little time to debug this, so I would recommend someone else review the visuazlization part.
Thanks again!

@kanishk16
Copy link
Contributor Author

kanishk16 commented Jun 6, 2022

No issues... IIUC the visualization is bound to fail when the backend is agg as visible here:

if matplotlib.get_backend() == "agg":
logger.warning("No backend can be used - Visualization will fail")
else:
logger.info(f"Using: {matplotlib.get_backend()} gui")

@kanishk16 kanishk16 changed the title fix: metric flag while vizualizing fix: metric flag while visualizing Jun 6, 2022
@lifetheater57
Copy link
Member

@kanishk16 How should I test this PR?

@kanishk16
Copy link
Contributor Author

kanishk16 commented Jun 13, 2022

@lifetheater57 My bad, I should've added the section how to test the PR earlier, nevertheless I've updated it.

@coveralls
Copy link

coveralls commented Jun 13, 2022

Pull Request Test Coverage Report for Build 3361273310

  • 1 of 11 (9.09%) changed or added relevant lines in 2 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 69.04%

Changes Missing Coverage Covered Lines Changed/Added Lines %
ivadomed/scripts/visualize_and_compare_testing_models.py 0 10 0.0%
Totals Coverage Status
Change from base Build 3361122051: 0.0%
Covered Lines: 4237
Relevant Lines: 6137

💛 - Coveralls

@naga-karthik
Copy link
Member

For some reason, I am getting this error on testing this PR:

what I ran: ivadomed_visualize_and_compare_testing_models --ofolders model1 model2 --metric dice_class0

Error Trace Screen Shot 2022-06-20 at 10 39 32 AM

I am not sure whether this is due to the PR (does seem like it though) and something do with my ivadomed version. Just a sanity check, I also ran the 2D UNet segmentation tutorial, and that started without any issues.
Any thoughts on what might be the issue here?

@kanishk16
Copy link
Contributor Author

Could you try adding the complete path for the model1 and model2?

@naga-karthik
Copy link
Member

Nope, getting the same error as above even with the full folder paths 😞

updated folder path Screen Shot 2022-06-20 at 11 02 31 AM

@kanishk16
Copy link
Contributor Author

Ahh, the error observed is the original one which seems to have fixed... It primarily stems from how the --metric flag parses the argument (#1138 (comment))

Could you check if the error persists on running the file as a script?

@naga-karthik
Copy link
Member

Sure! If you meant running it with python like (python visualize_and_compare_testing_models.py --ofolders ~/code/ivadomed/viz_test/model1 ~/code/ivadomed/viz_test/model2 --metric dice_class0) instead of ivadomed_visualize_and_compare_testing_models, then yes, I tried that and am still getting the same error.

What's surprising is that when I tried without the --metric argument (as suggested in the above PR comment), the code ran and I could see the output figure.

Copy link
Member

@naga-karthik naga-karthik left a comment

Choose a reason for hiding this comment

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

Just tested it, works fine me! Just a note though, the plot is not interactive. I can just see the plots.

@kanishk16
Copy link
Contributor Author

Idk why but the plots should be interactive... Why do I feel even those changes aren't there

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug category: fixes an error in the code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Metric and visualization bugs with visualize_and_compare_testing_models.py
5 participants