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

Suggestion: Add option to fail on reshape if a view could not be created #7292

Closed
DrChainsaw opened this issue Mar 12, 2019 · 4 comments

Comments

Projects
None yet
3 participants
@DrChainsaw
Copy link
Contributor

commented Mar 12, 2019

Issue Description

INDArray#reshape often but not always returns a view of the reshaped array and this makes it is easy to introduce hard to find bugs, especially since the method purpose is so clear the user might not read the javadoc.

Would it be helpful to have a method (or maybe flag) which indicates that the user prefers an exception over a copy if a view can not be created?

I understand this adds to an already huge API so I won't cry if you don't add it. If not ,what is a good way to check that you actually get a view? INDArray#isView()?

Version Information

Please indicate relevant versions, including, if relevant:

  • Deeplearning4j version 1.0.0-beta3

Aha! Link: https://skymindai.aha.io/features/ND4J-108

@AlexDBlack

This comment has been minimized.

Copy link
Member

commented Mar 12, 2019

I'm in favor of adding this. We already have this functionality anyway, but it's hidden from an API perspective:

public static INDArray newShapeNoCopy(INDArray arr, int[] newShape, boolean isFOrder) {
return newShapeNoCopy(arr, ArrayUtil.toLongArray(newShape), isFOrder);
}
/**
* A port of numpy's reshaping algorithm that leverages
* no copy where possible and returns
* null if the reshape
* couldn't happen without copying
* @param arr the array to reshape
* @param newShape the new shape
* @param isFOrder whether the array will be fortran ordered or not
* @return null if a reshape isn't possible, or a new ndarray
*/
public static INDArray newShapeNoCopy(INDArray arr, long[] newShape, boolean isFOrder) {

AlexDBlack added a commit that referenced this issue Mar 13, 2019

@AlexDBlack

This comment has been minimized.

Copy link
Member

commented Mar 13, 2019

Thanks for the suggestion. Implemented here, will be merged soon: #7293

@DrChainsaw

This comment has been minimized.

Copy link
Contributor Author

commented Mar 13, 2019

Awesome! Thanks alot!

@agibsonccc agibsonccc added the ND4J label Mar 18, 2019

AlexDBlack added a commit that referenced this issue Mar 18, 2019

[WIP] Various DL4J/ND4J fixes (#7293)
* Evaluation - segmentation support

* ROCBinary and ROCMultiClass segmentation support

* Fixes and EvaluationCalibration segmentation support

* ROC

* #7225 L2NormalizeVertex equals/hashCode fix

* #7268 Fix zeros constructor issue

* #7217 Fix (+ optimize) ConvolutionalIterationListener

* #7292 INDArray.reshape overload - enforce no view

* #7252 Remove Nd4j.emptyLike in favor of Nd4j.zerosLike

* #7277 Switch INDArray.repeat to native op to fix issue

* #7275 Nd4j.meshGrid dtype fix

* Switch Nd4j.tensorMmul to use custom op

* Small fixes

* Small fixes
@lock

This comment has been minimized.

Copy link

commented Apr 17, 2019

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@lock lock bot locked and limited conversation to collaborators Apr 17, 2019

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
You can’t perform that action at this time.