microsoft / microsoft/onnxruntime
`optimization.disable_specified_optimizers` is documented as comma-separated but is parsed on `;`
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
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- 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