Dynamic reasoning matcher

[bkorycki]

This is the first step towards handling reasoning SUTs dynamically. This PR specifically handles the case where a model returns reasoning by default and needs that reasoning parsed out. The user does not need to have any prior information about the model. Reasoning matching + handling will be done automatically.

The SUT factory now tries to match all SUTs against a set of (currently one) reasoning pattern. If it finds a match, it will dynamically mix that reasoning handler into the SUT.

There is a potential footgun; I had to remove the pre-defined thinking suts + mixins (e.g. TogetherThinkingSUT(TogetherSUT, ThinkingMixin)) because the code will dynamically match against against all concrete subclasses of ReasoningSUT. It should only match against the mix-ins themselves (e.g. ThinkingMixIn).

I removed all the pre-registered reasoning SUTs; they can now be accessed dynamically.

• deepseek-R1-thinking -> deepseek-ai/DeepSeek-R1:together
• moonshotai/Kimi-K2.5-together-thinking-excluded-hf -> moonshotai/Kimi-K2.5:together
  • This one doesn’t produce reasoning anyway. Maybe it did once?
• nvidia-nemotron-3-nano-30b-a-thinking-excluded-hf -> nvidia/NVIDIA-Nemotron-3-Nano-30B-A3B-BF16:hf
• PrimeIntellect-INTELLECT-3-thinking-excluded-hf -> PrimeIntellect/INTELLECT-3:hf

[github-actions]

MLCommons CLA bot All contributors have signed the MLCommons CLA ✍️ ✅

[bkorycki]

mypy was complaining about passing an abstract class to this method before.

[wpietri]

This seems very elaborate to me. I'm especially puzzled that there's a class called ReasoningSUT that is apparently not a SUT. But run-time patching of classes strikes me as a last-resort solution; it makes the system very hard to reason about. Could you say more about the simpler alternatives you considered and why you had to eliminate them?

[superdosh]

LGTM!

[superdosh]

Presumably the SUTs we've encountered don't have multiple think blocks?

[bkorycki]

They shouldn't! But I added some code to handle multiple think blocks just in case.

[superdosh]

I wonder if it might be organized better to put all this as a method in ReasoningSUT, and then you'd do something like: sut = ReasoningSUT.handle(sut) (which would be a no-op if internally find_match finds nothing).

[superdosh]

To William's point, organized this way, the class could end up being a DynamicReasoningSUTFactory?

[bkorycki]

Hm. I feel like replacing everything with sut = ReasoningSUT.handle(sut) makes it more difficult to understand/trace what's going on imho. But I'm open to this if you feel strongly that it would make it better organized!

I'm also not sure that creating a new factory would work in this case. This case is supposed to handle reasoning without the user having to explicitly know that the model is a reasoning model. To put this all in a different factory would require the user indicate that in the SUT UID.

[superdosh]

No strong opinions. Re: factory, I meant factory in a more general sense (something that produces the appropriately mixed in SUT) versus how we use SUTFactory, so my name was confusing.

[superdosh]

Somewhere, it would be good to document the fact that doing this check results in another call to the SUT.

[bkorycki]

@wpietri I can re-name ReasoningSUT to something like ReasoningHandler or ReasoningMixin if you'd like!

My other idea was to take make the Reasoning handler more of a wrapper that has the "base" sut as one of it's variables. But I thought that would be more complicated because I would have to re-write every sut method in the reasoning handler to call self.sut.method(), which seemed clunky and annoying to maintain if we add more functionality to the SUT class.

I'm happy to hear your thoughts on how we can simplify here!

[wpietri]

I'm happy to hear your thoughts on how we can simplify here!

Well let's try a domain-driven approach here. I like to start with a plain-language description of the domain and needs. How about I give a first pass just to show what I mean, and you can write a better version with your closer understanding here:

Our benchmark test certain LLMs, or things that have an interface like that. The things we test are called SUTs, systems under test. These are called prompt-response SUTs, because they accept prompts and return responses. Some of these SUTs have a "reasoning" mode, and some of those SUTs mix the "reasoning" text in with the output we want to test. For our benchmarks, we need to know if a SUT is a "reasoning" SUT, and if it's the kind that includes the "reasoning" text in the output, we want to strip that out for the benchmark.

What would you add and change there to make it better?

[bkorycki]

@wpietri I think your description is accurate! I would go on to say:

We decide if a SUT is a reasoning SUT by testing a single prompt-response to see if reasoning is included in the output. If it is, we need to add reasoning-handler capabilities to that SUT.

[wpietri]

@bkorycki Great! And how does a benchmark know a SUT is a reasoning SUT if the output isn't there?

[bkorycki]

@wpietri this PR only works with the output from model. If we don't have that, we would need some prior-information from the user about the model's reasoning capabilities. But I was trying to avoid that scenario in this PR and start somewhere simpler.

[wpietri]

@bkorycki Well if for now we aren't making reasoning-ness a property of a SUT and are just trying to make the benchmarks work, then how about we make a simple filter object and have the BenchmarkRunner use it on every response? Then we can avoid most of the complexity for now.

[bkorycki]

@wpietri That's an option. But personally I think that reasoning-related functionality should be a part of the SUT. I think testing a SUT with modelgauge run-sut or modelplane should invoke the same behavior as running that SUT in a benchmark.

Also, there is more to the reasoning handler than just parsing output. We also need to modify the token budgets in the request, which the reasoning class handles. I think it makes sense to keep all reasoning-related handling in one class.