remove batch from shape spec #18
Conversation
sheiksadique
changed the title
nirtorch/to_nir.py
Outdated
| @@ -49,7 +49,9 @@ def extract_nir_graph( | |||
|
|
|||
| # Convert the nodes and get indices | |||
| nir_edges = [] | |||
| nir_nodes = {"input": nir.Input(np.array(sample_data.shape))} | |||
| nir_nodes = { | |||
| "input": nir.Input(np.array(sample_data.shape[1:])) | |||
There was a problem hiding this comment.
would it make sense to add a flag is_batched and only remove the first dimension if this flag is true? I think we would always have batched input, so leaving it like this would also be fine with me
There was a problem hiding this comment.
In torch, the usual convention is to always have the batch dimension. So I would think it is safer to do this than to expect all other modules to add this flag of having a batch.
There was a problem hiding this comment.
I had to think about this for a bit. I don't think I understand the premise. Why do you have to modify the sample data? Can't the user just not include the batch dimension?
I'm asking because none of the PyTorch modules (linear, conv, ...) requires a batch dimension to evaluate them. Can't we just specify that whatever the user puts in, the user gets (with or without a batch dim)?
There was a problem hiding this comment.
Wasn't even aware this was possible! Alright I have an alternative solution, we can only look at the last necessary dimensions and ignore the other dims perhaps?
There was a problem hiding this comment.
Ok I see, we can't do that because we don't know what the dimensionality of the output or input is going to be in the first place.
@stevenabreu7 where do you suggest is_batched flag to go?
There was a problem hiding this comment.
I'm not sure what problem we're addressing at the moment. Can I ask why the shape of the input isn't sufficient? Wouldn't this be solved by something like extract_nir_graph(..., data.squeeze())?
|
Merged into #13 Nice work! |
Fixed input and output shapes generated in nirtorch to ignore batch shape.