gil icon indicating copy to clipboard operation
gil copied to clipboard

Add tests for checking boundary options in 1D convolution

Open meshtag opened this issue 4 years ago • 3 comments

This pull request intends to add unit tests for verifying boundary manipulation in 1D convolution. Since these are tests(used for proving correctness of implementation), I have favoured readability/simplicity over performance(for example : All tests written inside convolve.cpp do not require a separate image to be created after row convolution. However, doing so greatly simplifies its structure and saves the reader from unnecessary trouble of understanding a complex manipulation).

Flag boundary_option::extend_padded convolves the image assuming a padding around it. This leads to some unknown values of boundary pixels. For testing rest of the pixels(whose values can be predicted), I have deliberately made unknown values in original_output_image and expected_image equal. I have highlighted this with comments in the code.

Tasklist

  • [x] Add test case(s)
  • [ ] Ensure all CI builds pass
  • [ ] Review and approve

meshtag avatar May 29 '21 07:05 meshtag

Codecov Report

Merging #612 (368155c) into develop (843ea37) will increase coverage by 0.51%. The diff coverage is n/a.

@@             Coverage Diff             @@
##           develop     #612      +/-   ##
===========================================
+ Coverage    78.76%   79.28%   +0.51%     
===========================================
  Files          117      117              
  Lines         5030     5030              
===========================================
+ Hits          3962     3988      +26     
+ Misses        1068     1042      -26     

codecov[bot] avatar May 29 '21 08:05 codecov[bot]

I had a look at the error GA are giving. It seems like the compiler uses C++11 mode but standard library doesn't ... It seems more like C++20 is being used for standard library. I wanted to have a look at the CI script for GA, but couldn't find it. My guess off the bat would be that problem lies in the CMakeLists.txt explicitly setting CMAKE_CXX_STANDARD and GA passing it some other way which makes build select C++11 compiler mode and C++20 library.

simmplecoder avatar Jun 01 '21 16:06 simmplecoder

@meshtag Could you tell what is the status of this PR? Is this going to be picked up or drop it and close it?

mloskot avatar Jun 03 '22 06:06 mloskot