microsoft / microsoft/onnxruntime

`optimization.disable_specified_optimizers` is documented as comma-separated but is parsed on `;`

Open Beginner friendly
#32,211 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Dominant language
C++
Stars
21.9k
Forks
4.2k
Avg merge
4d 11h
Merged PRs (30d)
184

Description

### Describe the documentation issue

The repository contradicts itself about the separator for this session config value, and the failing case is silent.

**Documented as comma-separated** — `include/onnxruntime/core/session/onnxruntime_session_options_config_keys.h` (line 118 in `v1.29.0`):

> // This setting if set should contain a **comma separated** list of optimizers names that should be disabled.

**Parsed on `;`** — `onnxruntime/core/session/inference_session.cc` (line 503 in `v1.29.0`):

```cpp
const auto disabled_list = utils::SplitString(disabled_string, ";");
```

### Why it matters

A caller who follows the documentation and passes `"MatMulAddFusion,ConstantFolding"` gets a single optimizer name `MatMulAddFusion,ConstantFolding`, which matches nothing in the transformer registry. Nothing errors and nothing warns, so both optimizers stay enabled and the call silently does nothing — indistinguishable from a working call.

Passing a single optimizer name works either way, which is why this is easy to miss: it only bites the moment someone disables a second one.

### Suggested fix

Either the comment or the separator is wrong. Correcting the comment to say semicolon-separated looks like the cheaper and less breaking of the two. Optionally, rejecting (or warning on) a value that contains `,` but no `;` would catch the mistake at the call site, since that combination is almost always this error.

### Note on propagation

The wording has spread downstream. The Rust `ort` crate's `SessionBuilder::with_disabled_optimizers` repeats "Accepts a comma-separated list of optimizers to disable" verbatim (still present in its current release), forwarding the string unchanged to this config key. Fixing the wording here would let that follow.

### Page / URL

`include/onnxruntime/core/session/onnxruntime_session_options_config_keys.h` line 118, against `onnxruntime/core/session/inference_session.cc` line 503, both as of `v1.29.0`.

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Compare the setting comment in include/onnxruntime/core/session/onnxruntime_session_options_config_keys.h with the parsing in onnxruntime/core/session/inference_session.cc. Confirm that parsing uses a semicolon-separated value, then update the documented separator so the two locations agree. Done means the documented format matches the behavior described in the issue.

Written by the indexing model from the issue text.

Assessment

Tech stack
cpp
Domain
documentation
Issue type
Documentation
Difficulty
1/5
Estimated time
Under an hour
Activity status
Active
Clarity
Clearly specified
Newbie friendliness
88/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.