Skip to content

refactor: use sam3_graph_compute in sam2_encode_image_hiera #31

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.

Sign up for GitHub

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

Merged
PABannier merged 1 commit into from
Sep 16, 2026
Merged

refactor: use sam3_graph_compute in sam2_encode_image_hiera #31

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 1 addition & 5 deletions sam3.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5879,11 +5879,7 @@ static bool sam2_encode_image_hiera(sam3_state& state,
}

@PABannier PABannier Sep 16, 2026

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fix the merge conflicts

@PABannier PABannier Sep 16, 2026

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rebased onto main; conflict resolved and force-pushed. GitHub now reports the PR as mergeable.

The conflict was with #27 (RAII contexts/allocators), which had already deleted the manual ggml_gallocr_free(galloc); ggml_free(ctx0); lines this diff sat on top of. Resolution keeps #27's version, so the failure path is just return false;:

    // Compute
    if (!sam3_graph_compute(model.backend, graph, state.n_threads)) {
        return false;
    }

The diff is now 1 insertion, 5 deletions in sam2_encode_image_hiera — the thread-count setup, the status check and the duplicate error print, all of which sam3_graph_compute already does (it also logs the status string).

Verified: encode, PVS point + box, and a 3-frame visual track match main byte for byte on EdgeTAM f16 and SAM2.1-tiny q8_0, CPU and Metal (PARITY OK ×4); the SAM2.1 runs go through this function. A Release build with -DSAM3_BUILD_TESTS=ON has no errors or new warnings.

One note for later, left out to keep this diff as reviewed: sam3_encode_vit_from_preprocessed_selective (~sam3.cpp:7128) still open-codes the same pattern. It's in the test/debug region that #27 skipped, so it kept its manual frees and needs a slightly different edit.


// Compute
if (ggml_backend_is_cpu(model.backend)) {
ggml_backend_cpu_set_n_threads(model.backend, state.n_threads);
}
if (ggml_backend_graph_compute(model.backend, graph) != GGML_STATUS_SUCCESS) {
fprintf(stderr, "%s: graph compute failed\n", __func__);
if (!sam3_graph_compute(model.backend, graph, state.n_threads)) {
return false;
}

Expand Down
Loading