Skip to content

Conversation

@ChengYen-Tang
Copy link
Contributor

@NiklasGustafsson
Copy link
Contributor

NiklasGustafsson commented Apr 5, 2023

@ChengYen-Tang -- lots of good work here!! Could you please add a little bit more in terms of what this PR is for, and what scenarios it supports? I ask, because I don't see anything that's in the torch or torch.tensor scopes here.

@ChengYen-Tang
Copy link
Contributor Author

ChengYen-Tang commented Apr 6, 2023

@ChengYen-Tang -- lots of good work here!! Could you please add a little bit more in terms of what this PR is for, and what scenarios it supports? I ask, because I don't see anything that's in the torch or torch.tensor scopes here.

https://pytorch.org/docs/stable/tensorboard.html#torch.utils.tensorboard.writer.SummaryWriter.add_histogram
Required by the bins parameter in the tensorboard add_histogram method.

@NiklasGustafsson
Copy link
Contributor

Required by the bins parameter in the tensorboard add_histogram method.

Should it be internal rather than public, then?

@ChengYen-Tang
Copy link
Contributor Author

Required by the bins parameter in the tensorboard add_histogram method.

Should it be internal rather than public, then?

I think maybe others need this method too, this can be featured in torchsharp

@NiklasGustafsson
Copy link
Contributor

I think maybe others need this method too, this can be featured in torchsharp

Yes, I agree. In that case, I would start it out as internal. Changing it to public if there is demand won't be a breaking change.

@GeorgeS2019
Copy link

@ChengYen-Tang
Currently, I am occupied with other urgent tasks. Please kindly remove me from the reviewer list.

return new Tensor(res);
}

/// https://github.com/numpy/numpy/blob/v1.24.0/numpy/lib/histograms.py#L679
Copy link
Contributor

Choose a reason for hiding this comment

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

I know this is a little bit of a nit-pick, but does this method belong in a file called 'ComparisonOps'?

@NiklasGustafsson NiklasGustafsson merged commit 6fd563f into dotnet:main Apr 10, 2023
@ChengYen-Tang ChengYen-Tang deleted the histogram branch April 11, 2023 12:10
@GeorgeS2019
Copy link

GeorgeS2019 commented Apr 27, 2023

@ChengYen-Tang

Thank you very much.

Not being greedy, if you have time, a small polyglot notebook sample showing how to use the features for tensorboard you have written :-)

@ChengYen-Tang
Copy link
Contributor Author

Where to put it?
I have written an example here.
https://github.com/dotnet/TorchSharp/blob/main/src/Examples/Tensorboard.cs

@GeorgeS2019
Copy link

GeorgeS2019 commented Apr 27, 2023

@ChengYen-Tang

The aim is to show the community that What you do in PyTorch, the same can be done in TorchSharp.

Try to be as close as possible to this and turn that into dotnet PolyGlot notebook experience
https://pytorch.org/tutorials/recipes/recipes/tensorboard_with_pytorch.html
https://pytorch.org/tutorials/intermediate/tensorboard_tutorial.html <=== This could be BETTER

But use example where there is NOT too much extra codes to write, using pre-existing dataset and codes available in TorchSharp

@NiklasGustafsson

What do you think to put the polyglot notebook here?
https://github.com/dotnet/TorchSharpExamples/tree/main/tutorials/CSharp

@NiklasGustafsson
Copy link
Contributor

NiklasGustafsson commented Apr 27, 2023

Yes, I suggest adding another tutorial to the CSharp folder in dotnet/TorchSharpExamples. I can translate to F# if you aren't familiar with it. Maybe use a more useful name than what I have for the others? :-)

Make sure to follow the structure of the other tutorials -- the first cell is the same in all of them and sets up for automatic formatting of tensors as the output of a cell.

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.

3 participants