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

Updated viz_manager.py #169

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

ajinkya-kulkarni
Copy link

Modified the nested for loops that iterate over the image grid to use numpy vectorization instead, by reshaping the images and titles into 2D arrays, and then iterating over the first dimension of those arrays. This allows for faster iteration and better performance when dealing with large sets of images.

Modified the nested for loops that iterate over the image grid to use numpy vectorization instead, by reshaping the images and titles into 2D arrays, and then iterating over the first dimension of those arrays. 
This allows for faster iteration and better performance when dealing with large sets of images.
@CLAassistant
Copy link

CLAassistant commented May 5, 2023

CLA assistant check
All committers have signed the CLA.

@jwmueller jwmueller requested a review from sanjanag May 5, 2023 23:30
@jwmueller
Copy link
Member

@ajinkya-kulkarni Thanks for your awesome contribution!

Could you please sign the Contributor License Agreement linked above? Thank you!

@sanjanag
Copy link
Member

sanjanag commented May 9, 2023

Hi @ajinkya-kulkarni ! Thank you for the contribution. Could you post some results on performance gains and how the visualizations look like after this change?
Also, the PR is failing on some CI checks. Could you please fix these as well? These checks are important for testing and code maintenance. You can check the development guide for further details.
Feel free to let me know if you need anymore details.

@ajinkya-kulkarni
Copy link
Author

I will look into why is it failing.

@jwmueller
Copy link
Member

@ajinkya-kulkarni Any chance you can address the above comments from @sanjanag so we can get this PR in? Thanks!

@jwmueller
Copy link
Member

@ajinkya-kulkarni Any updates on this PR?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants