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

Add nrow and normalize args to TensorboardGenerativeModelImageSampler #489

Closed
chris-clem opened this issue Jan 3, 2021 · 5 comments · Fixed by #494
Closed

Add nrow and normalize args to TensorboardGenerativeModelImageSampler #489

chris-clem opened this issue Jan 3, 2021 · 5 comments · Fixed by #494
Assignees
Labels
callback enhancement New feature or request let's do it! Looking forward to have it implemented
Milestone

Comments

@chris-clem
Copy link
Contributor

🚀 Feature

Add nrow and normalize args to TensorboardGenerativeModelImageSampler

Motivation

At the moment, the image grid that is added to Tensorboard is created with grid = torchvision.utils.make_grid(images). It would be nice to have control over the nrow and normalize args of make_grid, e.g. through grid = torchvision.utils.make_grid(images, nrow=self.nrow, normalize=self.normalize).

Pitch

Change TensorboardGenerativeModelImageSampler's init from

    def __init__(self, num_samples: int = 3):
        super().__init__()
        self.num_samples = num_samples

to

    def __init__(self, num_samples: int = 3, nrow: int = 1, normalize: bool = True):
        super().__init__()
        self.num_samples = num_samples
        self.nrow = nrow
        self.normalize = normalize

and grid = torchvision.utils.make_grid(images) to grid = torchvision.utils.make_grid(images, nrow=self.nrow, normalize=self.normalize).

Alternatives

  • Add args for all of make_grid's args with sensible default values
  • Create a general image logging callback that can deal with all loggers and not just Tensorboard
@chris-clem chris-clem added enhancement New feature or request help wanted Extra attention is needed labels Jan 3, 2021
@chris-clem chris-clem changed the title Add nrowand normalizeargs to TensorboardGenerativeModelImageSampler Add nrow and normalize args to TensorboardGenerativeModelImageSampler Jan 3, 2021
@akihironitta akihironitta mentioned this issue Jan 3, 2021
8 tasks
@akihironitta
Copy link
Contributor

akihironitta commented Jan 3, 2021

@chris-clem Thank you for your suggestion! +1 for this change :]

It seems torchvision.utils.make_grid has other arguments as well, which we could possibly wrap them here. Do you think we should add other parameters, too, like padding, pad_value, ... of the function?

cc: @Borda @PyTorchLightning/bolts-contributors

@chris-clem
Copy link
Contributor Author

Yes, I think wrapping all parameters and using sensible defaults makes sense while we are already at it.

@carmocca
Copy link
Contributor

carmocca commented Jan 3, 2021

Makes sense, we can just pass **kwargs with the same default values.

@akihironitta
Copy link
Contributor

@chris-clem Would you be interested in submitting a PR?

@akihironitta akihironitta added let's do it! Looking forward to have it implemented and removed help wanted Extra attention is needed labels Jan 4, 2021
@chris-clem
Copy link
Contributor Author

@chris-clem Would you be interested in submitting a PR?

@akihironitta Yes, I will do it in the next days.

@akihironitta akihironitta self-assigned this Jan 5, 2021
@Borda Borda added this to the v0.3 milestone Jan 18, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
callback enhancement New feature or request let's do it! Looking forward to have it implemented
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants