Skip to content
Merged
Changes from 2 commits
Commits
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
28 changes: 28 additions & 0 deletions common/arg.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
#include "sampling.h"
#include "speculative.h"
#include "preset.h"
#include "gguf.h"

// fix problem with std::min and std::max
#if defined(_WIN32)
Expand Down Expand Up @@ -574,6 +575,33 @@ void common_models_handler_apply(common_models_handler & handler, common_params
}
}

// infer spec type from draft GGUF metadata when no sidecar or explicit type was given.
// NOTE: reads only the first split — DSpark drafters are single-file in practice
// (~11GB bf16, ~6.5GB Q2K), well below any split threshold.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

remove out-of-context comments

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

see agents.md for acceptable code comment style

if (spec_types_is_default(params) && !params.speculative.draft.mparams.path.empty()) {
struct gguf_init_params meta_params = {
/* .no_alloc = */ true,
/* .ctx = */ nullptr,
};
struct gguf_context * gguf_ctx = gguf_init_from_file(params.speculative.draft.mparams.path.c_str(), meta_params);

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

use RAII version of gguf_context in ggml-cpp.h

if (gguf_ctx) {
int64_t arch_idx = gguf_find_key(gguf_ctx, "general.architecture");
if (arch_idx >= 0) {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I believe the whole code should be part of speculative.cpp/.h

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Thanks for the detailed review, i agree with your points and have made the needed changes.

all three of ngxson's comments addressed in dd71b82, pushed to the PR branch:

  1. arg.cpp:589 "should be part of speculative.cpp/.h" → logic moved to new common_speculative_types_from_gguf() in common/speculative.cpp:2231; arg.cpp keeps a 7-line call site; #include "gguf.h" dropped from arg.cpp.
  2. arg.cpp:586 "use RAII" → now uses gguf_context_ptr from ggml-cpp.h; also added a gguf_get_kv_type string-type check so a malformed GGUF can't hit the gguf_get_val_str assert.
  3. arg.cpp:580 "out-of-context comments" / agents.md style → split-size NOTE deleted; one-line comment matching the adjacent sidecar block.

Verified: llama-common builds clean; runnable check against real models — DSpark drafter Q2K and bf16 → draft-dspark, vocab GGUF and missing file → empty (no crash).

std::string arch = gguf_get_val_str(gguf_ctx, arch_idx);
if (arch == "dflash") {
if (gguf_find_tensor(gguf_ctx, "markov_w1.weight") >= 0) {
params.speculative.types = { COMMON_SPECULATIVE_TYPE_DRAFT_DSPARK };
LOG_INF("common: auto-detected speculative type 'draft-dspark' from draft model (markov_w1.weight found)\n");
} else {
params.speculative.types = { COMMON_SPECULATIVE_TYPE_DRAFT_DFLASH };
LOG_INF("common: auto-detected speculative type 'draft-dflash' from draft model\n");
}
}
}
gguf_free(gguf_ctx);
}
}

// when a sidecar type is requested, the draft repo resolves to its sidecar instead of a full model
const bool spec_sidecar_found = !plan_spec.mtp.local_path.empty() ||
!plan_spec.dflash.local_path.empty() ||
Expand Down