-
Notifications
You must be signed in to change notification settings - Fork 33
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
Gwb grid limit resolution #653
Gwb grid limit resolution #653
Conversation
source/gwb-grid/main.cc
Outdated
@@ -363,7 +366,6 @@ int main(int argc, char **argv) | |||
number_of_threads = Utilities::string_to_unsigned_int(options_vector[i+1]); | |||
options_vector.erase(options_vector.begin()+static_cast<std::vector<std::string>::difference_type>(i)); | |||
options_vector.erase(options_vector.begin()+static_cast<std::vector<std::string>::difference_type>(i)); | |||
--i; |
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 no longer convinced this is correct. I am too tired to figure out.
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 vote for keeping --i;
tests/CMakeLists.txt
Outdated
@@ -256,7 +256,7 @@ file(GLOB_RECURSE COOKBOOK_TEST_SOURCES "${CMAKE_SOURCE_DIR}/cookbooks/*/*.wb") | |||
foreach(test_source ${COOKBOOK_TEST_SOURCES}) | |||
get_filename_component(test_name ${test_source} NAME_WE) | |||
get_filename_component(test_dir ${test_source} DIRECTORY) | |||
set(TEST_ARGUMENTS "${test_dir}/${test_name}.wb\;${test_dir}/${test_name}.grid") | |||
set(TEST_ARGUMENTS "${test_dir}/${test_name}.wb\;${test_dir}/${test_name}.grid\;--resolution-limit\;75") |
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.
20?
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 20 will be enough to ensure hitting any not huge feature in a model. 50?
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.
deal.
053479b
to
5eeb976
Compare
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## main #653 +/- ##
==========================================
+ Coverage 92.63% 92.68% +0.04%
==========================================
Files 98 98
Lines 6882 6888 +6
==========================================
+ Hits 6375 6384 +9
+ Misses 507 504 -3
... and 3 files with indirect coverage changes Continue to review full report in Codecov by Sentry.
|
|
This adds an option to limit the resolution of gwb-grid, and applies that to the cookbook tester.