-
-
Notifications
You must be signed in to change notification settings - Fork 332
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
i.biomass: added test file for i.biomass module #4962
base: main
Are you sure you want to change the base?
Conversation
) | ||
|
||
self.assertRasterFitsUnivar( | ||
raster=self.output_raster, reference=expected_stats, precision=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.
In other tests we use slightly lower precision which is sufficient. Also you can reduce the number of decimal places in the stats.
raster=self.output_raster, reference=expected_stats, precision=1e-8 | |
raster=self.output_raster, reference=expected_stats, precision=1e-6 |
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 have changed it to check upto six decimal places and also reduced the number of decimal places in the stats.
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.
The test functions can be executed in undefined order, so in the setUpClass you create the initial inputs, but the individual test functions modify them, so make sure that's not going to be a problem.
def setUp(self): | ||
"""Reset input rasters to default state before each test.""" | ||
input_expressions = { | ||
"fpar": "col() * 0.1", | ||
"lightuse_eff": "row() * 0.1", | ||
"latitude": "45.0", | ||
"dayofyear": "150", | ||
"transmissivity": "0.75", | ||
"water": "0.8", | ||
} | ||
self._create_input_rasters(input_expressions) | ||
|
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 think this setUp
is not a bad idea but given many tests are anyway creating their own layers, this is being run wastefully, so I would compute the data at the beginning of each test. We are trying to make the tests run fast.
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.
It's 5 seconds for now, some others in imagery take 1 second, some others take more.
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.
Thank you for the feedback! I have restructured the tests to address the feedback about efficiency. The setUp
method has been removed, and input rasters are now created directly within each test as needed avoiding redundant computations and ensuring faster test execution.
This PR introduces a suite of tests for the
i.biomass
GRASS GIS module, covering a range of scenarios, including functionality validation, property testing, edge cases, and performance evaluation.Key updates in this PR include:
Test Case Additions:
Basic Functionality:
Advanced Properties:
Edge Cases:
Performance: