-
Notifications
You must be signed in to change notification settings - Fork 6.7k
[MXNET-995] Constant Initializer for ND Array #12677
Conversation
|
@mxnet-label-bot [pr-awaiting-review] |
| if isinstance(source_array, NDArray): | ||
| dtype = source_array.dtype if dtype is None else dtype | ||
| else: | ||
| if isinstance(source_array, (float, int)): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
mx.base.numeric_types is better
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
my concern here is that you implicitly converted 0-d array(constant) to 1-d array here, which may not be very appropriate
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, my logic was since MXNet NDArray needs a 1d array minimum, I should convert constant. Else, is there a way I can do constant initialization support? Would be great to get some help in that direction.
| if source_array.shape != self.shape: | ||
| raise ValueError('Shape inconsistent: expected %s vs got %s'%( | ||
| str(self.shape), str(source_array.shape))) | ||
| str(source_array.shape), str(self.shape))) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why switch the order of expected vs got?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Have a look at #12676
I found it to be odd. Hence created an issue and fixed it in this PR
Don't you think so?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@zhreshold could you confirm if the above issue (#12676 ) is right or not? coz the value error it gave sounded confusing due to mismatched shapes. Thanks
|
Please check that this case was resolved by PR #12678 too. |
|
Ah, alright. So I'll take mine down then. Thanks for your contribution. |
Description
Allows 1D (constant) to be used to initialize an NDArray or Symbol
Fixes #12672 and #12676
Checklist
Essentials
Please feel free to remove inapplicable items for your PR.
Changes