[HIP] Add blender test #131
Conversation
| import numpy as np | ||
| from skimage.metrics import structural_similarity as ssim | ||
| except ImportError: | ||
| print("One or more required packages are not installed. Please install them using the command:") |
There was a problem hiding this comment.
Should that be checked during cmake configuration phase instead?
There was a problem hiding this comment.
Better to check in python script since the script may be used alone.
There was a problem hiding this comment.
You can have both. An enabled but failing test will just create unnecessary noise if tests are set up w/o blender. If blender is a requirement for the test suite, it's cmake's job to check for its presence, and enable/disable affected tests, IMO.
Up to you.
|
|
||
| mse_value = mse(image1, image2) | ||
|
|
||
| if ssim_index < ssim_threshold or mse_value > mse_threshold: |
There was a problem hiding this comment.
What exactly are we checking for here?
I assume that the goal is to make sure we produced a sensible image, but it does not have to be pixel-perfect?
I suspect these checks may be rather fragile in general, though it may work well enough for the specific image you use for the test. I guess for a single image we can always find a metric that works, even if blender results change in the future. Still, describing what we do here and why/how the metrics and thresholds were chosen may be helpful.
There was a problem hiding this comment.
ssim_index is 'structural similarity index' (https://en.wikipedia.org/wiki/Structural_similarity_index_measure), which is a popular measure of image similarity. Since it only measures gray-scale images, mse_value is also used, which measures 'mean square error' (https://en.wikipedia.org/wiki/Mean_squared_error). Together they should be a robust way to spot compiler issues without false alarms.
will add a comment about their meaning.
| continue | ||
| return data | ||
|
|
||
| def calculate_average(data): |
There was a problem hiding this comment.
The tests already depend on numpy. You could just use numpy to do the math,
There was a problem hiding this comment.
The script may be used alone. Since the calculation is trivial, it is unnecessary to introduce extra dependency.
| echo "TEST_SUITE_HIP_ROOT=$TEST_SUITE_HIP_ROOT" | ||
| ls $TEST_SUITE_HIP_ROOT | ||
| if [[ ! -e "$blender_dir/blender" || ! -e "$scene_dir/scenes.txt" ]]; then | ||
| echo "Skip HIP Blender test since no blender or test scenes found." |
There was a problem hiding this comment.
Another thing to check during cmake configuration?
| import numpy as np | ||
| from skimage.metrics import structural_similarity as ssim | ||
| except ImportError: | ||
| print("One or more required packages are not installed. Please install them using the command:") |
There was a problem hiding this comment.
You can have both. An enabled but failing test will just create unnecessary noise if tests are set up w/o blender. If blender is a requirement for the test suite, it's cmake's job to check for its presence, and enable/disable affected tests, IMO.
Up to you.
|
I'd like the blender test to be enabled in cmake file always so that the scripts are always available for testing. I will make change to the test script so that missing blender or Blender_Scenes will not cause error, but only skips the tests. |
yxsamliu
force-pushed
the
hip-add-blender-test2
branch
2 times, most recently
from
acff4bd to
e8adbc1
Compare
Add test scripts to run blender with run-time compilation of HIP kernels, which can be used to test clang/llvm built by buildbot.
yxsamliu
force-pushed
the
hip-add-blender-test2
branch
from
e8adbc1 to
f55ebfa
Compare
Add test scripts to run blender with run-time compilation of HIP kernels, which can be used to test clang/llvm built by buildbot.