-
Notifications
You must be signed in to change notification settings - Fork 8
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 image quality features #216
Conversation
JesseMckinzie
commented
Apr 23, 2024
- Add features for calculation image quality from https://github.com/hamshkhawar/image-tools/tree/quality/polus-image-quality-plugin
CMakeLists.txt
Outdated
@@ -258,7 +263,7 @@ if(CMAKE_CXX_COMPILER_ID STREQUAL "GNU") | |||
find_package(Threads QUIET) | |||
if (Threads_FOUND) | |||
if (CMAKE_USE_PTHREADS_INIT) | |||
SET(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -pthread") | |||
SET(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -pthread -g") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why are we adding debug flag here?
src/nyx/helpers/helpers.h
Outdated
} | ||
|
||
|
||
inline std::vector<int> arrange(int start, int end, int step=1) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should be replaced by std::copy
std::iota
src/nyx/helpers/helpers.h
Outdated
} | ||
|
||
|
||
inline std::vector<std::vector<int>> flipud (std::vector<std::vector<int>> vec) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
use std::reverse
?
@@ -91,7 +91,7 @@ std::vector<StatsReal> LR::get_fvals (int fcode) | |||
|
|||
void LR::initialize_fvals() | |||
{ | |||
fvals.resize ((int)Nyxus::Feature3D::_COUNT_); | |||
fvals.resize ((int)Nyxus::FeatureIMQ::_COUNT_); | |||
for (auto& valVec : fvals) | |||
valVec.push_back(0.0); | |||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am not sure if this is correct. At a given run, we other either computing 2D or 3D or IMQ features. In that case, should not Nyxus::Feature2D::_COUNT_
be sufficient (at the moment), since that's max number of features we are going to have at a given run?
@friskluft : Your thoughts?
|
||
TEST(TEST_NYXUS, TEST_PARQUET) { | ||
test_parquet(); | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Need to enable these back.
tests/test_feature_calculation.h
Outdated
#include "test_data.h" | ||
#include "test_main_nyxus.h" | ||
|
||
// check values of multiple features |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These should go inside namespace Nyxus
tests/test_feature_calculation.h
Outdated
void test_truth(const std::vector<double> values, const std::vector<double>& truth_values, double frac_tolerance) { | ||
|
||
if (values.size() != truth_values.size()) { | ||
FAIL() << "Number of truth values does not match the number of features calculated" << std::endl; | ||
} | ||
|
||
for (int i = 0; i < values.size(); ++i) { | ||
ASSERT_TRUE(agrees_gt(values[i], truth_values[i], frac_tolerance)); | ||
} | ||
} | ||
|
||
// check value of single feature | ||
void test_truth(const std::vector<double> values, double truth_value, double frac_tolerance) { | ||
ASSERT_TRUE(agrees_gt(values[0], truth_value, frac_tolerance)); | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If possible, make the two names different, since the action is different.
test.py
Outdated
from nyxus import Nyxus | ||
|
||
|
||
nyx = Nyxus(["FOCUS_SCORE"]) | ||
|
||
features = nyx.featurize_directory("/Users/mckinziejr/Documents/GitHub/nyxus/data/int", "/Users/mckinziejr/Documents/GitHub/nyxus/data/int") | ||
|
||
print(features) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is probably added by mistake
src/nyx/features/sharpness.h
Outdated
#include "../feature_method.h" | ||
#include "../environment.h" | ||
|
||
#define EPSILON 1e-8 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is already an EPSILON
variable in Nyxus somwhere. Use a constexpr
instead of macro
tests/test_feature_calculation.h
Outdated
namespace Nyxus | ||
{ | ||
// check value of single feature | ||
void test_truth(const std::vector<double> values, double truth_value, double frac_tolerance) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
not performance critical but this function should be like the following:
void test_truth(const double& value, double truth_value, double frac_tolerance){
ASSERT_TRUE(agrees_gt(value, truth_value, frac_tolerance));
}