-
Notifications
You must be signed in to change notification settings - Fork 6.8k
Conversation
@lanking520 could you help find someone to review this? thanks! |
@leleamol Could you please take a look? |
@chengzhengxin can you please upload the unit tests for this change? |
aux_arrays.push_back(nd); | ||
} else { | ||
NDArray nd = NDArray(aux_shapes[i], ctx); | ||
aux_arrays.push_back(nd); |
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.
can this line be moved to after the if-else instead of adding in if separately and else separately?
arg_arrays.push_back(nd); | ||
} else { | ||
NDArray nd = NDArray(arg_shapes[i], ctx); | ||
arg_arrays.push_back(nd); |
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.
can this line be moved to after the if-else instead of adding in if separately and else separately?
@mxnet-label-bot add[pr-awaiting-response] |
@mxnet-label-bot remove[pr-awaiting-review] |
@chengzhengxin can you please address the review comments? |
@leleamol Can you also have a look at this PR ? |
@chengzhengxin Could you address the comments to move forward with this PR? Thanks |
@chengzhengxin any updates? Could you please also add the unittest for the change? Thanks! |
@chengzhengxin Can you also rebase the PR with the latest master ? |
@chengzhengxin gentle ping for rebase and update |
sorry, more things have happened recently, It's too late to deal with it. But this bug has been fixed in #15245 |
Description
Load weights according dtype, it's ok for inference fp16 weights.