resolved issues in the comments

mindspore/ccsrc/dataset/kernels/image/random_crop_and_resize_op.cc

@@ -35,8 +35,10 @@ RandomCropAndResizeOp::RandomCropAndResizeOp(int32_t target_height, int32_t targ
     : target_height_(target_height),
       target_width_(target_width),
       rnd_scale_(scale_lb, scale_ub),
-      rnd_aspect_(aspect_lb, aspect_ub),
+      rnd_aspect_(log(aspect_lb), log(aspect_ub)),
       interpolation_(interpolation),
+      aspect_lb_(aspect_lb),
+      aspect_ub_(aspect_ub),
       max_iter_(max_iter) {
   rnd_.seed(GetSeed());
 }
@@ -64,33 +66,42 @@ Status RandomCropAndResizeOp::OutputShape(const std::vector<TensorShape> &inputs
   return Status(StatusCode::kUnexpectedError, "Input has a wrong shape");
 }
 Status RandomCropAndResizeOp::GetCropBox(int h_in, int w_in, int *x, int *y, int *crop_height, int *crop_width) {
-  double scale, aspect;
   *crop_width = w_in;
   *crop_height = h_in;
-  bool crop_success = false;
+  CHECK_FAIL_RETURN_UNEXPECTED(w_in != 0, "Width is 0");
+  CHECK_FAIL_RETURN_UNEXPECTED(h_in != 0, "Height is 0");
+  CHECK_FAIL_RETURN_UNEXPECTED(aspect_lb_ > 0, "Aspect lower bound must be greater than zero");
   for (int32_t i = 0; i < max_iter_; i++) {
-    scale = rnd_scale_(rnd_);
-    aspect = rnd_aspect_(rnd_);
-    *crop_width = static_cast<int32_t>(std::round(std::sqrt(h_in * w_in * scale / aspect)));
-    *crop_height = static_cast<int32_t>(std::round(*crop_width * aspect));
+    double const sample_scale = rnd_scale_(rnd_);
+    // In case of non-symmetrical aspect ratios, use uniform distribution on a logarithmic sample_scale.
+    // Note rnd_aspect_ is already a random distribution of the input aspect ratio in logarithmic sample_scale.
+    double const sample_aspect = exp(rnd_aspect_(rnd_));
+
+    *crop_width = static_cast<int32_t>(std::round(std::sqrt(h_in * w_in * sample_scale * sample_aspect)));
+    *crop_height = static_cast<int32_t>(std::round(*crop_width / sample_aspect));
     if (*crop_width <= w_in && *crop_height <= h_in) {
-      crop_success = true;
-      break;
+      std::uniform_int_distribution<> rd_x(0, w_in - *crop_width);
+      std::uniform_int_distribution<> rd_y(0, h_in - *crop_height);
+      *x = rd_x(rnd_);
+      *y = rd_y(rnd_);
+      return Status::OK();
     }
   }
-  if (!crop_success) {
-    CHECK_FAIL_RETURN_UNEXPECTED(w_in != 0, "Width is 0");
-    aspect = static_cast<double>(h_in) / w_in;
-    scale = rnd_scale_(rnd_);
-    *crop_width = static_cast<int32_t>(std::round(std::sqrt(h_in * w_in * scale / aspect)));
-    *crop_height = static_cast<int32_t>(std::round(*crop_width * aspect));
-    *crop_height = (*crop_height > h_in) ? h_in : *crop_height;
-    *crop_width = (*crop_width > w_in) ? w_in : *crop_width;
+  double const img_aspect = static_cast<double>(w_in) / h_in;
+  if (img_aspect < aspect_lb_) {
+    *crop_width = w_in;
+    *crop_height = static_cast<int32_t>(std::round(*crop_width / static_cast<double>(aspect_lb_)));
+  } else {
+    if (img_aspect > aspect_ub_) {
+      *crop_height = h_in;
+      *crop_width = static_cast<int32_t>(std::round(*crop_height * static_cast<double>(aspect_ub_)));
+    } else {
+      *crop_width = w_in;
+      *crop_height = h_in;
+    }
   }
-  std::uniform_int_distribution<> rd_x(0, w_in - *crop_width);
-  std::uniform_int_distribution<> rd_y(0, h_in - *crop_height);
-  *x = rd_x(rnd_);
-  *y = rd_y(rnd_);
+  *x = static_cast<int32_t>(std::round((w_in - *crop_width) / 2.0));
+  *y = static_cast<int32_t>(std::round((h_in - *crop_height) / 2.0));
   return Status::OK();
 }
 }  // namespace dataset

mindspore/ccsrc/dataset/kernels/image/random_crop_and_resize_op.h

@@ -60,6 +60,8 @@ class RandomCropAndResizeOp : public TensorOp {
   std::mt19937 rnd_;
   InterpolationMode interpolation_;
   int32_t max_iter_;
+  double aspect_lb_;
+  double aspect_ub_;
 };
 }  // namespace dataset
 }  // namespace mindspore

tests/ut/cpp/dataset/random_crop_and_resize_op_test.cc

@@ -28,17 +28,38 @@ class MindDataTestRandomCropAndResizeOp : public UT::CVOP::CVOpCommon {
  public:
   MindDataTestRandomCropAndResizeOp() : CVOpCommon() {}
 };
+TEST_F(MindDataTestRandomCropAndResizeOp, TestOpSimpleTest1) {
+  MS_LOG(INFO) << " starting RandomCropAndResizeOp simple test";
+  TensorShape s_in = input_tensor_->shape();
+  std::shared_ptr<Tensor> output_tensor;
+  int h_out = 1024;
+  int w_out = 2048;
+  float aspect_lb = 2;
+  float aspect_ub = 2.5;
+  float scale_lb = 0.2;
+  float scale_ub = 2.0;
 
-TEST_F(MindDataTestRandomCropAndResizeOp, TestOpSimpleTest) {
+  TensorShape s_out({h_out, w_out, s_in[2]});
+
+  auto op = std::make_unique<RandomCropAndResizeOp>(h_out, w_out, scale_lb, scale_ub, aspect_lb, aspect_ub);
+  Status s;
+  for (auto i = 0; i < 100; i++) {
+    s = op->Compute(input_tensor_, &output_tensor);
+    EXPECT_TRUE(s.IsOk());
+  }
+
+  MS_LOG(INFO) << "RandomCropAndResizeOp simple test finished";
+}
+TEST_F(MindDataTestRandomCropAndResizeOp, TestOpSimpleTest2) {
   MS_LOG(INFO) << " starting RandomCropAndResizeOp simple test";
   TensorShape s_in = input_tensor_->shape();
   std::shared_ptr<Tensor> output_tensor;
   int h_out = 1024;
   int w_out = 2048;
-  float aspect_lb = 0.2;
-  float aspect_ub = 5;
-  float scale_lb = 0.0001;
-  float scale_ub = 1.0;
+  float aspect_lb = 1;
+  float aspect_ub = 1.5;
+  float scale_lb = 0.2;
+  float scale_ub = 2.0;
 
   TensorShape s_out({h_out, w_out, s_in[2]});
 
@@ -51,3 +72,25 @@ TEST_F(MindDataTestRandomCropAndResizeOp, TestOpSimpleTest) {
 
   MS_LOG(INFO) << "RandomCropAndResizeOp simple test finished";
 }
+TEST_F(MindDataTestRandomCropAndResizeOp, TestOpSimpleTest3) {
+  MS_LOG(INFO) << " starting RandomCropAndResizeOp simple test";
+  TensorShape s_in = input_tensor_->shape();
+  std::shared_ptr<Tensor> output_tensor;
+  int h_out = 1024;
+  int w_out = 2048;
+  float aspect_lb = 0.2;
+  float aspect_ub = 3;
+  float scale_lb = 0.2;
+  float scale_ub = 2.0;
+
+  TensorShape s_out({h_out, w_out, s_in[2]});
+
+  auto op = std::make_unique<RandomCropAndResizeOp>(h_out, w_out, scale_lb, scale_ub, aspect_lb, aspect_ub);
+  Status s;
+  for (auto i = 0; i < 100; i++) {
+    s = op->Compute(input_tensor_, &output_tensor);
+    EXPECT_TRUE(s.IsOk());
+  }
+
+  MS_LOG(INFO) << "RandomCropAndResizeOp simple test finished";
+}
\ No newline at end of file

tests/ut/python/dataset/test_random_crop_and_resize.py

@@ -41,7 +41,8 @@ def test_random_crop_and_resize_op_c(plot=False):
     # First dataset
     data1 = ds.TFRecordDataset(DATA_DIR, SCHEMA_DIR, columns_list=["image"], shuffle=False)
     decode_op = c_vision.Decode()
-    random_crop_and_resize_op = c_vision.RandomResizedCrop((256, 512), (1, 1), (0.5, 0.5))
+    # With these inputs we expect the code to crop the whole image
+    random_crop_and_resize_op = c_vision.RandomResizedCrop((256, 512), (2, 2), (1, 3))
     data1 = data1.map(input_columns=["image"], operations=decode_op)
     data1 = data1.map(input_columns=["image"], operations=random_crop_and_resize_op)
 
@@ -65,6 +66,7 @@ def test_random_crop_and_resize_op_c(plot=False):
     if plot:
         visualize(original_images, crop_and_resize_images)
 
+
 def test_random_crop_and_resize_op_py(plot=False):
     """
     Test RandomCropAndResize op in py transforms
@@ -72,9 +74,10 @@ def test_random_crop_and_resize_op_py(plot=False):
     logger.info("test_random_crop_and_resize_op_py")
     # First dataset
     data1 = ds.TFRecordDataset(DATA_DIR, SCHEMA_DIR, columns_list=["image"], shuffle=False)
+    # With these inputs we expect the code to crop the whole image
     transforms1 = [
         py_vision.Decode(),
-        py_vision.RandomResizedCrop((256, 512), (1, 1), (0.5, 0.5)),
+        py_vision.RandomResizedCrop((256, 512), (2, 2), (1, 3)),
         py_vision.ToTensor()
     ]
     transform1 = py_vision.ComposeOp(transforms1)
@@ -96,6 +99,8 @@ def test_random_crop_and_resize_op_py(plot=False):
         original = (item2["image"].transpose(1, 2, 0) * 255).astype(np.uint8)
         original = cv2.resize(original, (512, 256))
         mse = diff_mse(crop_and_resize, original)
+        # Due to rounding error the mse for Python is not exactly 0
+        assert mse <= 0.05
         logger.info("random_crop_and_resize_op_{}, mse: {}".format(num_iter + 1, mse))
         num_iter += 1
         crop_and_resize_images.append(crop_and_resize)
@@ -103,6 +108,7 @@ def test_random_crop_and_resize_op_py(plot=False):
     if plot:
         visualize(original_images, crop_and_resize_images)
 
+
 def test_random_crop_and_resize_01():
     """
     Test RandomCropAndResize with md5 check, expected to pass
@@ -114,7 +120,7 @@ def test_random_crop_and_resize_01():
     # First dataset
     data1 = ds.TFRecordDataset(DATA_DIR, SCHEMA_DIR, columns_list=["image"], shuffle=False)
     decode_op = c_vision.Decode()
-    random_crop_and_resize_op = c_vision.RandomResizedCrop((256, 512), (0.5, 1), (0.5, 1))
+    random_crop_and_resize_op = c_vision.RandomResizedCrop((256, 512), (0.5, 0.5), (1, 1))
     data1 = data1.map(input_columns=["image"], operations=decode_op)
     data1 = data1.map(input_columns=["image"], operations=random_crop_and_resize_op)
 
@@ -122,7 +128,7 @@ def test_random_crop_and_resize_01():
     data2 = ds.TFRecordDataset(DATA_DIR, SCHEMA_DIR, columns_list=["image"], shuffle=False)
     transforms = [
         py_vision.Decode(),
-        py_vision.RandomResizedCrop((256, 512), (0.5, 1), (0.5, 1)),
+        py_vision.RandomResizedCrop((256, 512), (0.5, 0.5), (1, 1)),
         py_vision.ToTensor()
     ]
     transform = py_vision.ComposeOp(transforms)
@@ -137,6 +143,7 @@ def test_random_crop_and_resize_01():
     ds.config.set_seed(original_seed)
     ds.config.set_num_parallel_workers(original_num_parallel_workers)
 
+
 def test_random_crop_and_resize_02():
     """
     Test RandomCropAndResize with md5 check:Image interpolation mode is Inter.NEAREST,
@@ -172,6 +179,7 @@ def test_random_crop_and_resize_02():
     ds.config.set_seed(original_seed)
     ds.config.set_num_parallel_workers(original_num_parallel_workers)
 
+
 def test_random_crop_and_resize_03():
     """
     Test RandomCropAndResize with md5 check: max_attempts is 1, expected to pass
@@ -206,6 +214,7 @@ def test_random_crop_and_resize_03():
     ds.config.set_seed(original_seed)
     ds.config.set_num_parallel_workers(original_num_parallel_workers)
 
+
 def test_random_crop_and_resize_04_c():
     """
     Test RandomCropAndResize with c_tranforms: invalid range of scale (max<min),
@@ -225,6 +234,7 @@ def test_random_crop_and_resize_04_c():
         logger.info("Got an exception in DE: {}".format(str(e)))
         assert "Input range is not valid" in str(e)
 
+
 def test_random_crop_and_resize_04_py():
     """
     Test RandomCropAndResize with py_transforms: invalid range of scale (max<min),
@@ -247,6 +257,7 @@ def test_random_crop_and_resize_04_py():
         logger.info("Got an exception in DE: {}".format(str(e)))
         assert "Input range is not valid" in str(e)
 
+
 def test_random_crop_and_resize_05_c():
     """
     Test RandomCropAndResize with c_transforms: invalid range of ratio (max<min),
@@ -266,6 +277,7 @@ def test_random_crop_and_resize_05_c():
         logger.info("Got an exception in DE: {}".format(str(e)))
         assert "Input range is not valid" in str(e)
 
+
 def test_random_crop_and_resize_05_py():
     """
     Test RandomCropAndResize with py_transforms: invalid range of ratio (max<min),
@@ -288,6 +300,7 @@ def test_random_crop_and_resize_05_py():
         logger.info("Got an exception in DE: {}".format(str(e)))
         assert "Input range is not valid" in str(e)
 
+
 def test_random_crop_and_resize_comp(plot=False):
     """
     Test RandomCropAndResize and compare between python and c image augmentation
@@ -321,6 +334,7 @@ def test_random_crop_and_resize_comp(plot=False):
     if plot:
         visualize(image_c_cropped, image_py_cropped)
 
+
 if __name__ == "__main__":
     test_random_crop_and_resize_op_c(True)
     test_random_crop_and_resize_op_py(True)