🧪 Add tests for softmax_naive in ml_kernels

[bugparty]

🎯 What: Added missing tests for the softmax_naive function in the ml_kernels subproject. Created ml_kernels/src/kernel_test.cpp and wired it into the build system.
📊 Coverage: Covered the standard happy path (calculating softmax and verifying probabilities sum to 1), empty input edge case, and large negative inputs for numerical stability verification.
✨ Result: Improved test coverage for ml_kernels, adding a reliable and isolated unit test built using standard C++ asserts.

---

PR created automatically by Jules for task 5571039690984798118 started by @bugparty

Summary by CodeRabbit

• Tests
  • Added a new standalone test executable that validates softmax behavior with basic, empty-input, and negative-value edge-case tests; prints a success message on pass.
• Chores
  • Updated build configuration so the new test executable is built and receives the same release optimization settings as existing test targets.

[google-labs-jules]

👋 Jules, reporting for duty! I'm here to lend a hand with this pull request.

When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down.

I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job!

For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with @jules. You can find this option in the Pull Request section of your global Jules UI settings. You can always switch back!

New to Jules? Learn more at jules.google/docs.

---

For security, I will only act on instructions from the user who triggered this task.

[coderabbitai]

Warning

Rate limit exceeded

@bugparty has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 56 minutes and 33 seconds before requesting another review.

Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 56 minutes and 33 seconds.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 7abce164-6ec4-4ca6-a83f-b47029d6a43a

📥 Commits

Reviewing files that changed from the base of the PR and between 7245e53 and 3ca149e.

📒 Files selected for processing (18)

• .github/workflows/benchmark_gemm.yml
• .github/workflows/build.yml
• .jules/thunderbolt.md
• CMakeLists.txt
• dgetrf/CMakeLists.txt
• dgetrf/lapack.c
• dgetrf/my.c
• dgetrf/test_swapRow.c
• gemm/dgemm7v2.c
• gemm/dgemm7v2.cpp
• gemm/transpose_2level.h
• ml_kernels/CMakeLists.txt
• ml_kernels/include/ml_kernels/softmax.h
• ml_kernels/src/kernel_bench.cpp
• ml_kernels/src/kernel_test.cpp
• ml_kernels/src/test_naive_ops.cpp
• mpi_prime/func_call.c
• mpi_prime/sieve3.cpp

📝 Walkthrough

Walkthrough

Adds a new CMake executable target ml_kernel_test_softmax and a new test source ml_kernels/src/kernel_test.cpp that exercises ml_kernels::softmax_naive with three test cases (basic, empty input, negative-values).

Changes

Cohort / File(s)	Summary
Build Configuration ml_kernels/CMakeLists.txt	Adds new executable target ml_kernel_test_softmax built from src/naive_ops.cpp and src/kernel_test.cpp; ensures the same release/non-Debug optimization flags as existing kernel test/bench targets (/O2 on MSVC, -O3 -march=native otherwise).
Test Implementation ml_kernels/src/kernel_test.cpp	New standalone test executable implementing three sequential tests for softmax_naive: basic correctness (length-3 input), empty-input (size 0, no crash), and negative-values (equal large-magnitude negatives); prints success message on pass.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

🐇 I hopped through code to check the sum,
Three little tests to see them run,
Empty, negative, and plain and neat,
Softmax proved on every seat,
A tiny hop — the kernels won! 🎉

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly describes the main change: adding tests for the softmax_naive function in ml_kernels. However, it includes an emoji (🧪) which adds noise.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch test-improvement-softmax-naive-5571039690984798118

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[bugparty]

@copilot resolve the merge conflicts in this pull request

[bugparty]

@codex[agent] fix the merge conflicts

[chatgpt-codex-connector]

To use Codex here, create an environment for this repo.

[bugparty]

@copilot resolve the merge conflicts in this pull request

[Copilot]

@copilot resolve the merge conflicts in this pull request

Resolved and pushed. I merged origin/main, fixed the conflict in ml_kernels/CMakeLists.txt, and validated the build/tests. Addressed in commit 9eaafee (merge resolution commit: 7245e53).

[coderabbitai]

Actionable comments posted: 2

🧹 Nitpick comments (1)

ml_kernels/src/kernel_test.cpp (1)

41-56: Make the numerical-stability case non-uniform.

Using three identical large negatives only expects 1/3 for every output. A buggy implementation that underflows and falls back to a uniform distribution could still pass; use distinct large-negative values and compare against max-shifted expected probabilities.

Example adjustment

 void test_softmax_naive_negative() {
-    std::vector<float> input = {-1000.0f, -1000.0f, -1000.0f};
+    std::vector<float> input = {-1000.0f, -1001.0f, -1002.0f};
     std::vector<float> output(3, 0.0f);
 
     ml_kernels::softmax_naive(input.data(), output.data(), input.size());
 
+    float max_val = -1000.0f;
+    float sum_exp = std::exp(input[0] - max_val) + std::exp(input[1] - max_val) + std::exp(input[2] - max_val);
+    float expected_0 = std::exp(input[0] - max_val) / sum_exp;
+    float expected_1 = std::exp(input[1] - max_val) / sum_exp;
+    float expected_2 = std::exp(input[2] - max_val) / sum_exp;
+
     float sum = 0.0f;
     for (float val : output) {
         sum += val;
     }
 
-    // Since they are equal, each should be 1/3
     assert(std::abs(sum - 1.0f) < 1e-5f);
-    assert(std::abs(output[0] - (1.0f / 3.0f)) < 1e-5f);
-    assert(std::abs(output[1] - (1.0f / 3.0f)) < 1e-5f);
-    assert(std::abs(output[2] - (1.0f / 3.0f)) < 1e-5f);
+    assert(std::abs(output[0] - expected_0) < 1e-5f);
+    assert(std::abs(output[1] - expected_1) < 1e-5f);
+    assert(std::abs(output[2] - expected_2) < 1e-5f);
 }

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@ml_kernels/src/kernel_test.cpp` around lines 41 - 56, The test
test_softmax_naive_negative uses three identical large negatives which can hide
underflow bugs; change the input vector to distinct large-negative values (e.g.,
-1000, -1001, -1002) and compute the expected probabilities by subtracting the
max (max-shift), applying exp, and normalizing to sum to 1, then assert each
output from ml_kernels::softmax_naive matches the computed expected
probabilities within the same tolerance; update assertions to compare output[i]
against the max-shifted expected[i] and still verify the outputs sum to 1.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@ml_kernels/CMakeLists.txt`:
- Around line 29-34: The new test executables (ml_kernel_test_softmax,
ml_kernel_test_naive_ops, ml_kernel_test) are built but not registered with
CTest; add add_test() registrations for each target so ctest will run them
(e.g., add_test(NAME ml_kernel_test_softmax COMMAND ml_kernel_test_softmax)),
and ensure testing is enabled (enable_testing()) if not already; place the
add_test calls after the corresponding add_executable/target_link_libraries
blocks so CTest will discover and execute the targets.

In `@ml_kernels/src/kernel_test.cpp`:
- Around line 27-30: Replace the fragile assert(...) checks in kernel_test.cpp
with unconditional runtime checks that cannot be compiled out: for each
assert(std::abs(sum - 1.0f) < 1e-5f) and the asserts on
output[0]/output[1]/output[2] vs expected_0/expected_1/expected_2, change them
to explicit if-statements that check the same condition and, on failure, print a
clear error (including the actual and expected values and the tolerance) to
stderr and terminate the test with a non-zero exit (e.g., std::cerr << ...;
std::exit(1) or throw std::runtime_error). This ensures checks on symbols sum,
output, expected_0, expected_1, expected_2 always run regardless of NDEBUG.

---

Nitpick comments:
In `@ml_kernels/src/kernel_test.cpp`:
- Around line 41-56: The test test_softmax_naive_negative uses three identical
large negatives which can hide underflow bugs; change the input vector to
distinct large-negative values (e.g., -1000, -1001, -1002) and compute the
expected probabilities by subtracting the max (max-shift), applying exp, and
normalizing to sum to 1, then assert each output from ml_kernels::softmax_naive
matches the computed expected probabilities within the same tolerance; update
assertions to compare output[i] against the max-shifted expected[i] and still
verify the outputs sum to 1.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: b8e8b30e-d657-441e-b16b-108e0cdf179b

📥 Commits

Reviewing files that changed from the base of the PR and between a27315d and 7245e53.

📒 Files selected for processing (2)

• ml_kernels/CMakeLists.txt
• ml_kernels/src/kernel_test.cpp

[bugparty]

@copilot Verify each finding against the current code and only fix it if needed.

In @ml_kernels/CMakeLists.txt around lines 29 - 34, The new test executables
(ml_kernel_test_softmax, ml_kernel_test_naive_ops, ml_kernel_test) are built but
not registered with CTest; add add_test() registrations for each target so ctest
will run them (e.g., add_test(NAME ml_kernel_test_softmax COMMAND
ml_kernel_test_softmax)), and ensure testing is enabled (enable_testing()) if
not already; place the add_test calls after the corresponding
add_executable/target_link_libraries blocks so CTest will discover and execute
the targets.
Verify each finding against the current code and only fix it if needed.

In @ml_kernels/src/kernel_test.cpp around lines 27 - 30, Replace the fragile
assert(...) checks in kernel_test.cpp with unconditional runtime checks that
cannot be compiled out: for each assert(std::abs(sum - 1.0f) < 1e-5f) and the
asserts on output[0]/output[1]/output[2] vs expected_0/expected_1/expected_2,
change them to explicit if-statements that check the same condition and, on
failure, print a clear error (including the actual and expected values and the
tolerance) to stderr and terminate the test with a non-zero exit (e.g.,
std::cerr << ...; std::exit(1) or throw std::runtime_error). This ensures checks
on symbols sum, output, expected_0, expected_1, expected_2 always run regardless
of NDEBUG.

[Copilot]

@copilot Verify each finding against the current code and only fix it if needed.

In @ml_kernels/CMakeLists.txt around lines 29 - 34, The new test executables
...

Implemented and pushed in db20fd6. I verified the current state and applied only needed fixes: added CTest registrations for existing test targets (ml_kernel_test, ml_kernel_test_softmax) and replaced the softmax assert checks in kernel_test.cpp with explicit runtime checks that print detailed stderr output and exit non-zero on failure. Note: ml_kernel_test_naive_ops is not present in the current code, so no registration was added for that non-existent target. No UI changes in this PR (screenshot: N/A).