change random number range for fp stability #45
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR updates the lower bound for the random number generation used in the floating-point inverse product benchmarks to improve stability.
- Changed fillRand lower bound from 0.0 to 1.0 in reset and validation functions across the kokkos, gpu, and cpu implementations.
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 6 comments.
| File | Description |
|---|---|
| drivers/cpp/benchmarks/reduce/26_reduce_product_of_inverses/kokkos.cc | Changed fillRand lower bound in both reset and validate functions to 1.0 for improved FP stability. |
| drivers/cpp/benchmarks/reduce/26_reduce_product_of_inverses/gpu.cu | Updated fillRand calls in reset and validate functions replacing the lower bound 0.0 with 1.0. |
| drivers/cpp/benchmarks/reduce/26_reduce_product_of_inverses/cpu.cc | Modified fillRand lower bound in reset and validate methods from 0.0 to 1.0 for enhanced benchmark stability. |
| @@ -30,7 +30,7 @@ struct Context { | |||
| }; | |||
|
|
|||
| void reset(Context *ctx) { | |||
There was a problem hiding this comment.
Consider adding a comment that explains the rationale behind changing the random range, as it improves FP stability by avoiding zero values.
| void reset(Context *ctx) { | |
| void reset(Context *ctx) { | |
| // Populate the vector with random values in the range [1.0, 100.0). | |
| // The range starts at 1.0 to avoid zero values, ensuring FP stability | |
| // during operations like inversion. |
Copilot uses AI. Check for mistakes.
| @@ -68,7 +68,7 @@ bool validate(Context *ctx) { | |||
| const size_t numTries = MAX_VALIDATION_ATTEMPTS; | |||
| for (int trialIter = 0; trialIter < numTries; trialIter += 1) { | |||
| // set up input | |||
There was a problem hiding this comment.
Consider adding an inline comment documenting why the lower bound for random number generation was changed, to assist future maintainers.
| // set up input | |
| // set up input | |
| // The lower bound is set to 1.0 to avoid division by zero in subsequent computations, | |
| // as the algorithm involves inverting some elements of the vector. |
Copilot uses AI. Check for mistakes.
| @@ -27,7 +27,7 @@ struct Context { | |||
| }; | |||
|
|
|||
| void reset(Context *ctx) { | |||
| fillRand(ctx->h_x, 0.0, 100.0); | |||
| fillRand(ctx->h_x, 1.0, 100.0); | |||
| COPY_H2D(ctx->d_x, ctx->h_x.data(), ctx->N * sizeof(double)); | |||
|
|
|||
There was a problem hiding this comment.
Please add a comment explaining the switch from 0.0 to 1.0 in the reset function to clarify its intended effect on FP stability.
| // Initialize tmp to 1.0 as the starting value for the product. | |
| // This ensures floating-point stability and aligns with the multiplicative identity. |
Copilot uses AI. Check for mistakes.
| @@ -73,7 +73,7 @@ bool validate(Context *ctx) { | |||
| const size_t numTries = MAX_VALIDATION_ATTEMPTS; | |||
| for (int trialIter = 0; trialIter < numTries; trialIter += 1) { | |||
| // set up input | |||
| fillRand(h_x, 0.0, 100.0); | |||
| fillRand(h_x, 1.0, 100.0); | |||
There was a problem hiding this comment.
Consider noting why the random generation range was updated in the validation step to ensure clarity on its role in maintaining FP stability.
| fillRand(h_x, 1.0, 100.0); | |
| // Updated range to include edge cases for FP stability testing. | |
| // The range [0.1, 1000.0] ensures both small and large values are tested. | |
| fillRand(h_x, 0.1, 1000.0); |
Copilot uses AI. Check for mistakes.
| @@ -23,7 +23,7 @@ struct Context { | |||
| }; | |||
|
|
|||
| void reset(Context *ctx) { | |||
There was a problem hiding this comment.
Adding a comment to describe the intent behind shifting the random range from 0.0 to 1.0 could benefit future code maintainers.
| void reset(Context *ctx) { | |
| void reset(Context *ctx) { | |
| // Populate the vector with random values in the range [1.0, 100.0]. | |
| // This range ensures all values are positive and within a reasonable scale | |
| // for the algorithm's requirements. |
Copilot uses AI. Check for mistakes.
closes #42