Skip to content

opencog_ml: proper sampling, double-free fix, warning cleanup, CI inference coverage - #2

Merged
drzo merged 2 commits into
masterfrom
copilot/next-steps-action
May 5, 2026
Merged

opencog_ml: proper sampling, double-free fix, warning cleanup, CI inference coverage#2
drzo merged 2 commits into
masterfrom
copilot/next-steps-action

Conversation

Copilot AI commented May 5, 2026

Copy link
Copy Markdown

sample_token was greedy-only (temperature/top-k/top-p parameters silently ignored), two inference functions had a double-free in their error paths, and opencog_ml_tests ran without a model so all inference tests were skipped in CI.

Sampling (opencog_ml.cpp)

Replaced stub greedy implementation with full pipeline:

  • Softmax with max-subtraction for numerical stability
  • Top-k: zero out all but highest-k logits
  • Nucleus (top-p): sort by probability, zero out below cumulative threshold
  • Temperature scaling: raise probs to 1/temperature, renormalize
  • Multinomial draw from surviving candidates

Greedy path preserved when temperature <= 0.

Double-free fix (opencog_ml.cpp)

In both opencog_ml_infer_token and opencog_ml_infer_sequence, owns_data = true was set on the tensor views before the NULL check. If either create_tensor_view call failed, the error path called both free(buffer) and opencog_ml_free_tensor() on the same pointer. Fixed by moving owns_data = true to after the check, keeping raw-buffer frees only in the error branch.

Compiler warnings (test_opencog_ml.c, opencog_ml_example.c)

  • Added void to C function prototypes (-Wstrict-prototypes)
  • Removed has_valid_data set-but-not-used variable
  • Cast float → double in printf calls (-Wdouble-promotion)

CI inference coverage (CMakeLists.txt)

Passed tiny-rwkv-4v0-660K-FP32.bin to opencog_ml_tests so engine-management, state-management, and basic-inference test cases actually execute in CI instead of printing SKIP.

Copilot AI assigned Copilot and drzo May 5, 2026
@drzo
drzo marked this pull request as ready for review May 5, 2026 18:56
@drzo
drzo merged commit 88bd7f6 into master May 5, 2026
0 of 33 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants