Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add tests for checking boundary options in 1D convolution #612

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from

Conversation

meshtag
Copy link
Member

@meshtag meshtag commented May 29, 2021

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

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

@meshtag meshtag added google-summer-of-code All items related to GSoC activities test New tests development or missing tests issues, no new functionality labels May 29, 2021
@codecov
Copy link

codecov bot commented May 29, 2021

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     

test/extension/numeric/convolve.cpp Outdated Show resolved Hide resolved
test/extension/numeric/convolve.cpp Outdated Show resolved Hide resolved
test/extension/numeric/convolve.cpp Outdated Show resolved Hide resolved
test/extension/numeric/convolve.cpp Outdated Show resolved Hide resolved
test/extension/numeric/convolve.cpp Outdated Show resolved Hide resolved
test/extension/numeric/convolve.cpp Outdated Show resolved Hide resolved
@simmplecoder
Copy link
Contributor

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.

@mloskot
Copy link
Member

mloskot commented Jun 3, 2022

@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 mloskot added status/need-feedback Asking for more details about the problem status/work-in-progress Do NOT merge yet until this label has been removed! labels Jun 3, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
google-summer-of-code All items related to GSoC activities status/need-feedback Asking for more details about the problem status/work-in-progress Do NOT merge yet until this label has been removed! test New tests development or missing tests issues, no new functionality
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants