added horizontal and vertical flip augmentations #38
Conversation
|
Thanks for opening your first pull request in this repository! Someone will review it when they have a chance. In the mean time, please be sure that you've handled the following things, to make the review process quicker and easier:
Thank you again for your contributions! 👍 |
augmentation/augmentation_impl.hpp
Outdated
| { | ||
| this->VerticalFlipTransform(dataset, datapointWidth, datapointHeight, | ||
| datapointDepth, augmentations[i]); | ||
| } | ||
| else |
There was a problem hiding this comment.
Here I am adding directly in the for loop because I am not able to use augmentationMap. I tried to use like -
https://stackoverflow.com/questions/14419202/c-map-of-string-and-member-function-pointer but I am getting error. So if can suggest How should I use it? How should I store the function in the map?
There was a problem hiding this comment.
Hmm, I will give detailed review in a day or two. There are some more changes that we would need to make.
There was a problem hiding this comment.
Could you also share the exact error. I will take a look. Thanks.
|
Hmm, I don't think we need horizontal-flip:true or false. If user wants it he will add to the string else he won't. |
augmentation/augmentation_impl.hpp
Outdated
| bool ishortiflip = false; | ||
| // Get ishortiflip. | ||
| GetHorizontalFlipParam(ishortiflip, augmentation); | ||
| // if(!ishortiflip) return ; |
There was a problem hiding this comment.
I will uncomment it. I have added the comment by mistake.
Okay, I will the change code in the next commit. |
augmentation/augmentation.hpp
Outdated
| /** | ||
| * Function to determine if augmentation has horizontal-flip function. | ||
| * | ||
| * @param augmentation Optional argument to check if a string has | ||
| * horizontal-flip substring. | ||
| */ | ||
| bool HasHorizontalFlipParam(const std::string& augmentation = "") | ||
| { | ||
| if (augmentation.length()) | ||
| return augmentation.find("horizontal-flip") != std::string::npos; | ||
|
|
||
|
|
||
| // Search in augmentation vector. | ||
| for(size_t i=0; i<augmentations.size(); i++) | ||
| { | ||
| if(augmentations[i].find("horizontal-flip") != std::string::npos) | ||
| return true; | ||
| } | ||
| return false; | ||
|
|
||
| } |
There was a problem hiding this comment.
I don't think we need this if we use a map instead.
augmentation/augmentation.hpp
Outdated
| /** | ||
| * Function to determine if augmentation has vertical-flip function. | ||
| * | ||
| * @param augmentation Optional argument to check if a string has | ||
| * vertical-flip substring. | ||
| */ | ||
| bool HasVerticalFlipParam(const std::string& augmentation = "") | ||
| { | ||
| if (augmentation.length()) | ||
| return augmentation.find("vertical-flip") != std::string::npos; | ||
|
|
||
|
|
||
| // Search in augmentation vector. | ||
| for(size_t i=0; i<augmentations.size(); i++) | ||
| { | ||
| if(augmentations[i].find("vertical-flip") != std::string::npos) | ||
| return true; | ||
| } | ||
| return false; | ||
|
|
||
| } |
augmentation/augmentation.hpp
Outdated
| void GetHorizontalFlipParam(bool& ishortiflip, | ||
| const std::string& augmentation) | ||
| { | ||
| if (!HasHorizontalFlipParam()) | ||
| return; | ||
|
|
||
| ishortiflip = false; | ||
|
|
||
| // Use regex to find true or false. | ||
| boost::regex regex{"(?:true|false)"}; | ||
|
|
||
| // Create an iterator to find matches. | ||
| boost::sregex_token_iterator matches(augmentation.begin(), | ||
| augmentation.end(), regex, 0), end; | ||
|
|
||
| size_t matchesCount = std::distance(matches, end); | ||
|
|
||
| if (matchesCount == 1) | ||
| { | ||
| ishortiflip = (*matches) == "true" ? true:false; | ||
| } | ||
| else | ||
| { | ||
| mlpack::Log::Fatal << "Invalid boolean value in " << | ||
| augmentation << std::endl; | ||
| } | ||
| } |
There was a problem hiding this comment.
If we remove the need for true and false, assume we need to flip if there is horizontal flip then we can remove this as well.
augmentation/augmentation.hpp
Outdated
| * @param isvertiflip Output is verticalr flipped or not. | ||
| * @param augmentation String from boolean value is extracted. | ||
| */ | ||
| void GetVerticalFlipParam(bool& isvertiflip, |
There was a problem hiding this comment.
Hey @RituRajSingh878, There are some changes that we need to make here. Let me know what you think. Thanks for working on this. Once we fix that error and remove the unnecessary part of the code we should be able to merge this.
Thanks.
for(size_t i = 0; i < augmentations.size(); i++)
{
if( augmentations[i] == "horizontal-flip")
augmentationMap[augmentations[i]] = &Augmentation::HorizontalFlipTransform;
}I am doing smothing like this and getting the following error. I am not sure how should I use add it and where should I add it? I guess, I am doing something wrong and don't know how to use it properly. So if you can explain and suggest something. home/rituraj/models/tests/augmentation_tests.cpp:32:63: required from here
/home/rituraj/models/tests/../augmentation/augmentation_impl.hpp:32:39: error: no matches converting function ‘HorizontalFlipTransform’ to type ‘std::unordered_map<std::__cxx11::basic_string<char>, void (*)(arma::Mat<double>&, long unsigned int, long unsigned int, long unsigned int, std::__cxx11::basic_string<char>&), std::hash<std::__cxx11::basic_string<char> >, std::equal_to<std::__cxx11::basic_string<char> >, std::allocator<std::pair<const std::__cxx11::basic_string<char>, void (*)(arma::Mat<double>&, long unsigned int, long unsigned int, long unsigned int, std::__cxx11::basic_string<char>&)> > >::mapped_type {aka void (*)(class arma::Mat<double>&, long unsigned int, long unsigned int, long unsigned int, class std::__cxx11::basic_string<char>&)}’
augmentationMap[augmentations[i]] = &Augmentation::HorizontalFlipTransform;
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~
In file included from /home/rituraj/models/tests/augmentation_tests.cpp:14:0:
/home/rituraj/models/tests/../augmentation/augmentation.hpp:124:8: note: candidate is: template<class DatasetType> void Augmentation::HorizontalFlipTransform(DatasetType&, size_t, size_t, size_t, const string&)
void HorizontalFlipTransform(DatasetType& dataset,
^~~~~~~~~~~~~~~~~~~~~~~
tests/CMakeFiles/models_test.dir/build.make:62: recipe for target 'tests/CMakeFiles/models_test.dir/augmentation_tests.cpp.o' failed
make[2]: *** [tests/CMakeFiles/models_test.dir/augmentation_tests.cpp.o] Error 1
make[2]: *** Waiting for unfinished jobs.... |
|
This issue has been automatically marked as stale because it has not had any recent activity. It will be closed in 7 days if no further activity occurs. Thank you for your contributions! 👍 |
|
This issue has been automatically marked as stale because it has not had any recent activity. It will be closed in 7 days if no further activity occurs. Thank you for your contributions! 👍 |
Related to #21
I am adding horizontal-flip and vertical-flip augmentations in augmentation. I am opening this as a draft pr because many things are not clear to me and have to discuss it. After that, I will modify it.