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

log a few images to tensorboard #485

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

Conversation

notiho
Copy link
Contributor

@notiho notiho commented Apr 6, 2023

No description provided.

Copy link
Contributor

@bencomp bencomp left a comment

Choose a reason for hiding this comment

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

Thanks for making my comment in #482 into this PR. The code looks even more understandable than I expected :) (I haven't tested it myself, but I assume it works.)

I wonder if it makes sense to log the number of the sample in the train_set, i.e. the result of np.random.randint instead of or next to the i, so that it can be looked up more easily? If the train_set is shuffled or if the dataset is an Arrow file, then this number would not map to an image that you can look up easily.

Perhaps the logging of images could be made optional too, by adding a CLI option that sets the max number of logged images to 0 by default. You could refer to that number instead of 16.

@PonteIneptique
Copy link
Contributor

Just an additional comment: this would be a wonderful thing to also log the prediction with it :)
But either, thank you for this PR !

@mittagessen
Copy link
Owner

I wonder if it makes sense to log the number of the sample in the train_set, i.e. the result of np.random.randint instead of or next to the i, so that it can be looked up more easily? If the train_set is shuffled or if the dataset is an Arrow file, then this number would not map to an image that you can look up easily.

Because the samples might be substituted on the fly if loading them fails for any reason the indices might not actually correspond to the order of the dataset. And as a note you can unpack a binary dataset with contrib/extract_lines.py now but if you're defining fixed splits in the file the indices in the extracted file names won't match the ones used during training even in the absence of errors.

@notiho
Copy link
Contributor Author

notiho commented Apr 7, 2023

I now also added logging for validation images and their prediction. How do you feel about adding another parameter to control the image logging?

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