MXPredReshape bug: need to reshape softmax_label

[chsin]

Description

The fix in #11493 allows reshape only for networks that don't use SoftmaxOutput, so none of the pretrained image classification networks, e.g. inception, can be reshaped. The current test did not catch this because it only used gluon blocks. The reason SoftmaxOutput is weird is because it requires the label "softmax_label" to be reshaped even though "softmax_label" is only used as an input for training and should not even be looked at for prediction, which has been mentioned in other context before. I don't know what's the best way to deal with SoftmaxOutput's label reshape requirement because I can't find a way to get the label names. I am sending this pull request with an edit that allows resnet and inception to be reshaped, along with a test that covers this edge case, but I hope someone can comment on a more robust solution.

I found this issue when I made the following edits to image-classification-predict.cc:223. Before this change, the following would print out -1 and I traced it to c_predict_api.cc:304 where newShape.Size() != arr.shape().Size() for when arg_names[I] is softmax_label:

  // Create Predictor
  PredictorHandle old_hnd = nullptr;
  const mx_uint old_input_shape_data[4] = { 2,
                                        static_cast<mx_uint>(channels),
                                        static_cast<mx_uint>(height),
                                        static_cast<mx_uint>(width) };
  MXPredCreate(static_cast<const char*>(json_data.GetBuffer()),
               static_cast<const char*>(param_data.GetBuffer()),
               static_cast<int>(param_data.GetLength()),
               dev_type,
               dev_id,
               num_input_nodes,
               input_keys,
               input_shape_indptr,
               old_input_shape_data,
               &old_hnd);
  assert(old_hnd);

  int e = MXPredReshape(num_input_nodes,
                        input_keys,
                        input_shape_indptr,
                        input_shape_data,
                        old_hnd,
                        &pred_hnd);
  printf("%d\n", e);

Checklist

Essentials

Please feel free to remove inapplicable items for your PR.

• Changes are complete (i.e. I finished coding on this PR)
• All changes have test coverage:
• Unit tests are added for small changes to verify correctness (e.g. adding a new operator)
• Nightly tests are added for complicated/long-running ones (e.g. changing distributed kvstore)
• Build tests will be added for build configuration changes (e.g. adding a new build option with NCCL)
• Code is well-documented:
• For user-facing API changes, API doc string has been updated.
• For new C++ functions in header files, their functionalities and arguments are documented.
• For new examples, README.md is added to explain the what the example does, the source of the dataset, expected performance on test set and reference to the original paper if applicable
• Check the API doc at http://mxnet-ci-doc.s3-accelerate.dualstack.amazonaws.com/PR-$PR_ID/$BUILD_ID/index.html
• To the my best knowledge, examples are either not affected by this change, or have been fixed to be compatible with this change

[chinakook]

In the inference stage, it's no need to use labels or SoftmaxOutput.

[chinakook]

You can write a script to deploy the symbol by replace SoftmaxOutput with SoftmaxActivate.

[chsin]

@chinakook, To predict using the C API, you use MXPredCreate to load the pretrained symbols and weights and this works fine for all the mxnet pretrained models that use SoftmaxOutput, but the problem is that MXPredReshape doesn't work even though the models that were loaded properly. This causes reshape to fail for all the pretrained mxnet models (almost all use SoftmaxOutput) even though predcreate is fine. This is not an acceptable situation. I don't quite understand your comment, but you seem to be suggesting that this inconsistency is acceptable coding practice and it is the user that needs to clean up this mess by writing a script?

[lupesko]

Thanks for identifying the issue and filing this PR @chsin .
Looping in additional folks to review: @apeforest @anirudh2290 @eric-haibin-lin

[stu1130]

bounce again @apeforest @anirudh2290 @eric-haibin-lin

[vandanavk]

@apeforest @anirudh2290 @eric-haibin-lin ping for review

[vandanavk]

@mxnet-label-bot [pr-awaiting-review]

[apeforest]

Still trying to understand the problem. But this hack is not acceptable purely from coding practice.

[Roshrini]

@chsin Can you please address review comments? Thanks!

[ankkhedia]

@chsin ping! Could you please address the comments? Thanks!

[kalyc]

@chsin Thanks for your contribution! We would like to take the process of merging this PR forward - could you please address comments above?

[stu1130]

@mxnet-label-bot update [pr-awaiting-response]
@chsin let us know if you need any help!

[vandanavk]

adding @Vikas89 for review

[roywei]

@chsin Thanks for the contribution, looks like our CI hanged, could trigger it by push an empty commit?

[sandeep-krishnamurthy]

@chsin - Can you please rebase and retrigger CI?

[Roshrini]

@anirudh2290 Can you review this PR?

[stu1130]

@anirudh2290 ping for review @chsin could you rebase and retrigger CI thanks!

[sandeep-krishnamurthy]

@chsin - Thanks for your contributions. Like @apeforest mentioned, this change looks like not generalizable. What happens if name of last layer is not 'softmax_label' for example in Gluon case it will may be prefixed with name_scope.