Skip to content
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.

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

Fix: MiniMax model function call #1116

Closed

Conversation

mxsl-gr
Copy link
Contributor

@mxsl-gr mxsl-gr commented Jul 25, 2024

PR content:
fix MiniMax model function call and unit tests.

Most scenarios has worked, but in some complex situations within the English context, if there are multiple rounds of function calls, there's a certain probability they might stop prematurely.
For example, when trying to get the weather for multiple locations at once, but the function for getting the weather only accepts a single location as a parameter. This is mainly limited by the model's performance.

related issue: #1077

@tzolov
Copy link
Contributor

tzolov commented Jul 31, 2024

HI @mxsl-gr and thank you for the update.
As we are deprecating AbstractFunctionCallSupport in favor of AbstractToolCallSupport would you be interested to re-work your PR to support AbstractToolCallSupport?

For examples, you can check the OpenAI/AzureOpenAI/Anthropic/Mistral/Ollama/VertexAIGemini function calling implementations, or ask me for support ;)
Would appreciate your help as you own those models.

@mxsl-gr
Copy link
Contributor Author

mxsl-gr commented Aug 1, 2024

hi, no problem, i'll check and handle them later

@mxsl-gr mxsl-gr force-pushed the fix/minimax-function-call branch 2 times, most recently from a058546 to aa1030f Compare August 5, 2024 01:14
@mxsl-gr
Copy link
Contributor Author

mxsl-gr commented Aug 5, 2024

hi, @tzolov
it worked for now, and this PR has been squash and force pushed

@@ -126,7 +127,7 @@ public void toolFunctionCall() {

assertThat(chatCompletion2.getBody().choices().get(0).message().role()).isEqualTo(Role.ASSISTANT);
assertThat(chatCompletion2.getBody().choices().get(0).message().content()).contains("San Francisco")
.containsAnyOf("30.0°C", "30°C", "30.0°F", "30°F");
.containsAnyOf("30.0°C", "30°C");
Copy link
Member

Choose a reason for hiding this comment

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

I had to update to use

				.containsAnyOf("30.0°C", "30°C", "30.0")
				.containsAnyOf("°C", "Celsius");

as the answer said "30 Celcius"

also, it seems sometimes i get an answer back in chinese. Is there a way for it to be force to reply in english?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

as far as i know, there's can't to set the talk language through options, but we can add an explicit request in the message, like please answer in english

@@ -78,11 +79,12 @@ void functionCallTest() {
@Test
void streamingFunctionCallTest() {
Copy link
Member

Choose a reason for hiding this comment

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

This test fails with

java.lang.RuntimeException: com.fasterxml.jackson.databind.exc.MismatchedInputException: No content to map due to end-of-input
 at [Source: REDACTED (`StreamReadFeature.INCLUDE_SOURCE_IN_LOCATION` disabled); line: 1]

	at org.springframework.ai.model.function.AbstractFunctionCallback.fromJson(AbstractFunctionCallback.java:115)
	at org.springframework.ai.model.function.AbstractFunctionCallback.call(AbstractFunctionCallback.java:104)
	at org.springframework.ai.model.function.FunctionCallbackWrapper.call(FunctionCallbackWrapper.java:35)
	at org.springframework.ai.chat.model.AbstractToolCallSupport.executeFunctions(AbstractToolCallSupport.java:200)
	at org.springframework.ai.chat.model.AbstractToolCallSupport.handleToolCalls(AbstractToolCallSupport.java:139)
	at org.springframework.ai.minimax.MiniMaxChatModel.lambda$stream$6(MiniMaxChatModel.java:247)
	at reactor.core.publisher.FluxFlatMap$FlatMapMain.onNext(FluxFlatMap.java:388)
	at reactor.core.publisher.FluxSwitchMapNoPrefetch$SwitchMapInner.onNext(FluxSwitchMapNoPrefetch.java:408)
	at reactor.core.publisher.FluxMapFuseable$MapFuseableSubscriber.onNext(FluxMapFuseable.java:129)
	at reactor.core.publisher.Operators$ScalarSubscription.request(Operators.java:2571)
	at reactor.core.publisher.FluxMapFuseable$MapFuseableSubscriber.request(FluxMapFuseable.java:171)
	at reactor.core.publisher.FluxSwitchMapNoPrefetch$SwitchMapInner.onSubscribe(FluxSwitchMapNoPrefetch.java:378)
	at reactor.core.publisher.FluxMapFuseable$MapFuseableSubscriber.onSubscribe(FluxMapFuseable.java:96)
	at reactor.core.publisher.MonoJust.subscribe(MonoJust.java:55)
	at reactor.core.publisher.Mono.subscribe(Mono.java:4568)
	at reactor.core.publisher.FluxSwitchMapNoPrefetch$SwitchMapMain.subscribeInner(FluxSwitchMapNoPrefetch.java:219)
	at reactor.core.publisher.FluxSwitchMapNoPrefetch$SwitchMapMain.onNext(FluxSwitchMapNoPrefetch.java:193)
	at reactor.core.publisher.FluxMap$MapSubscriber.onNext(FluxMap.java:122)
	at reactor.core.publisher.FluxFlatMap$FlatMapMain.tryEmit(FluxFlatMap.java:547)
	at reactor.core.publisher.FluxFlatMap$FlatMapInner.onNext(FluxFlatMap.java:988)
	at reactor.core.publisher.Operators$BaseFluxToMonoOperator.completePossiblyEmpty(Operators.java:2097)
	at reactor.core.publisher.MonoReduceSeed$ReduceSeedSubscriber.onComplete(MonoReduceSeed.java:163)
	at reactor.core.publisher.FluxWindowPredicate$WindowFlux.checkTerminated(FluxWindowPredicate.java:768)
	at reactor.core.publisher.FluxWindowPredicate$WindowFlux.drainRegular(FluxWindowPredicate.java:662)
	at reactor.core.publisher.FluxWindowPredicate$WindowFlux.drain(FluxWindowPredicate.java:748)
	at reactor.core.publisher.FluxWindowPredicate$WindowFlux.onComplete(FluxWindowPredicate.java:814)
	at reactor.core.publisher.FluxWindowPredicate$WindowPredicateMain.onNext(FluxWindowPredicate.java:243)
	at reactor.core.publisher.FluxMap$MapSubscriber.onNext(FluxMap.java:122)
	at reactor.core.publisher.FluxMap$MapSubscriber.onNext(FluxMap.java:122)
	at reactor.core.publisher.FluxFilter$FilterSubscriber.onNext(FluxFilter.java:113)
	at reactor.core.publisher.FluxTakeUntil$TakeUntilPredicateSubscriber.onNext(FluxTakeUntil.java:95)
	at reactor.core.publisher.MonoFlatMapMany$FlatMapManyInner.onNext(MonoFlatMapMany.java:251)
	at reactor.core.publisher.FluxOnErrorResume$ResumeSubscriber.onNext(FluxOnErrorResume.java:79)
	at reactor.core.publisher.FluxOnAssembly$OnAssemblySubscriber.onNext(FluxOnAssembly.java:539)
	at reactor.core.publisher.FluxConcatMapNoPrefetch$FluxConcatMapNoPrefetchSubscriber.innerNext(FluxConcatMapNoPrefetch.java:259)
	at reactor.core.publisher.FluxConcatMap$ConcatMapInner.onNext(FluxConcatMap.java:865)
	at reactor.core.publisher.FluxConcatMap$WeakScalarSubscription.request(FluxConcatMap.java:480)
	at reactor.core.publisher.Operators$MultiSubscriptionSubscriber.set(Operators.java:2367)
	at reactor.core.publisher.FluxConcatMapNoPrefetch$FluxConcatMapNoPrefetchSubscriber.onNext(FluxConcatMapNoPrefetch.java:202)
	at reactor.core.publisher.FluxBufferPredicate$BufferPredicateSubscriber.onNextNewBuffer(FluxBufferPredicate.java:317)
	at reactor.core.publisher.FluxBufferPredicate$BufferPredicateSubscriber.tryOnNext(FluxBufferPredicate.java:227)
	at reactor.core.publisher.FluxBufferPredicate$BufferPredicateSubscriber.onNext(FluxBufferPredicate.java:200)
	at reactor.core.publisher.FluxPeekFuseable$PeekFuseableConditionalSubscriber.onNext(FluxPeekFuseable.java:503)
	at reactor.core.publisher.FluxMapFuseable$MapFuseableConditionalSubscriber.onNext(FluxMapFuseable.java:299)
	at reactor.core.publisher.FluxContextWrite$ContextWriteSubscriber.onNext(FluxContextWrite.java:107)
	at reactor.core.publisher.FluxDoFinally$DoFinallySubscriber.onNext(FluxDoFinally.java:113)
	at reactor.core.publisher.FluxConcatArray$ConcatArraySubscriber.onNext(FluxConcatArray.java:180)
	at reactor.core.publisher.FluxFlattenIterable$FlattenIterableSubscriber.drainAsync(FluxFlattenIterable.java:453)
	at reactor.core.publisher.FluxFlattenIterable$FlattenIterableSubscriber.drain(FluxFlattenIterable.java:724)
	at reactor.core.publisher.FluxFlattenIterable$FlattenIterableSubscriber.onNext(FluxFlattenIterable.java:256)
	at reactor.core.publisher.FluxMap$MapSubscriber.onNext(FluxMap.java:122)
	at reactor.core.publisher.FluxPeek$PeekSubscriber.onNext(FluxPeek.java:200)
	at reactor.core.publisher.FluxMap$MapSubscriber.onNext(FluxMap.java:122)
	at reactor.netty.channel.FluxReceive.drainReceiver(FluxReceive.java:294)
	at reactor.netty.channel.FluxReceive.onInboundNext(FluxReceive.java:403)
	at reactor.netty.channel.ChannelOperations.onInboundNext(ChannelOperations.java:426)

@@ -103,14 +106,16 @@ void functionCallWithPortableFunctionCallingOptions() {
});
}

// FIXME: multiple function calls may stop prematurely due to model performance
@Test
void streamFunctionCallTest() {
Copy link
Member

Choose a reason for hiding this comment

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

This test fails with

java.lang.RuntimeException: com.fasterxml.jackson.databind.exc.MismatchedInputException: No content to map due to end-of-input
 at [Source: REDACTED (`StreamReadFeature.INCLUDE_SOURCE_IN_LOCATION` disabled); line: 1]

	at org.springframework.ai.model.function.AbstractFunctionCallback.fromJson(AbstractFunctionCallback.java:115)
	at org.springframework.ai.model.function.AbstractFunctionCallback.call(AbstractFunctionCallback.java:104)
	at org.springframework.ai.model.function.FunctionCallbackWrapper.call(FunctionCallbackWrapper.java:35)
	at org.springframework.ai.chat.model.AbstractToolCallSupport.executeFunctions(AbstractToolCallSupport.java:200)
	at org.springframework.ai.chat.model.AbstractToolCallSupport.handleToolCalls(AbstractToolCallSupport.java:139)
	at org.springframework.ai.minimax.MiniMaxChatModel.lambda$stream$6(MiniMaxChatModel.java:247)
	at reactor.core.publisher.FluxFlatMap$FlatMapMain.onNext(FluxFlatMap.java:388)
	at reactor.core.publisher.FluxSwitchMapNoPrefetch$SwitchMapInner.onNext(FluxSwitchMapNoPrefetch.java:408)
	at reactor.core.publisher.FluxMapFuseable$MapFuseableSubscriber.onNext(FluxMapFuseable.java:129)
	at reactor.core.publisher.Operators$ScalarSubscription.request(Operators.java:2571)
	at reactor.core.publisher.FluxMapFuseable$MapFuseableSubscriber.request(FluxMapFuseable.java:171)
	at reactor.core.publisher.FluxSwitchMapNoPrefetch$SwitchMapInner.onSubscribe(FluxSwitchMapNoPrefetch.java:378)
	at reactor.core.publisher.FluxMapFuseable$MapFuseableSubscriber.onSubscribe(FluxMapFuseable.java:96)
	at reactor.core.publisher.MonoJust.subscribe(MonoJust.java:55)
	at reactor.core.publisher.Mono.subscribe(Mono.java:4568)
	at reactor.core.publisher.FluxSwitchMapNoPrefetch$SwitchMapMain.subscribeInner(FluxSwitchMapNoPrefetch.java:219)
	at reactor.core.publisher.FluxSwitchMapNoPrefetch$SwitchMapMain.onNext(FluxSwitchMapNoPrefetch.java:193)
	at reactor.core.publisher.FluxMap$MapSubscriber.onNext(FluxMap.java:122)
	at reactor.core.publisher.FluxFlatMap$FlatMapMain.tryEmit(FluxFlatMap.java:547)
	at reactor.core.publisher.FluxFlatMap$FlatMapInner.onNext(FluxFlatMap.java:988)
	at reactor.core.publisher.Operators$BaseFluxToMonoOperator.completePossiblyEmpty(Operators.java:2097)
	at reactor.core.publisher.MonoReduceSeed$ReduceSeedSubscriber.onComplete(MonoReduceSeed.java:163)
	at reactor.core.publisher.FluxWindowPredicate$WindowFlux.checkTerminated(FluxWindowPredicate.java:768)
	at reactor.core.publisher.FluxWindowPredicate$WindowFlux.drainRegular(FluxWindowPredicate.java:662)
	at reactor.core.publisher.FluxWindowPredicate$WindowFlux.drain(FluxWindowPredicate.java:748)
	at reactor.core.publisher.FluxWindowPredicate$WindowFlux.onComplete(FluxWindowPredicate.java:814)
	at reactor.core.publisher.FluxWindowPredicate$WindowPredicateMain.onNext(FluxWindowPredicate.java:243)
	at reactor.core.publisher.FluxMap$MapSubscriber.onNext(FluxMap.java:122)
	at reactor.core.publisher.FluxMap$MapSubscriber.onNext(FluxMap.java:122)
	at reactor.core.publisher.FluxFilter$FilterSubscriber.onNext(FluxFilter.java:113)
	at reactor.core.publisher.FluxTakeUntil$TakeUntilPredicateSubscriber.onNext(FluxTakeUntil.java:95)
	at reactor.core.publisher.MonoFlatMapMany$FlatMapManyInner.onNext(MonoFlatMapMany.java:251)
	at reactor.core.publisher.FluxOnErrorResume$ResumeSubscriber.onNext(FluxOnErrorResume.java:79)
	at reactor.core.publisher.FluxOnAssembly$OnAssemblySubscriber.onNext(FluxOnAssembly.java:539)
	at reactor.core.publisher.FluxConcatMapNoPrefetch$FluxConcatMapNoPrefetchSubscriber.innerNext(FluxConcatMapNoPrefetch.java:259)
	at reactor.core.publisher.FluxConcatMap$ConcatMapInner.onNext(FluxConcatMap.java:865)
	at reactor.core.publisher.FluxConcatMap$WeakScalarSubscription.request(FluxConcatMap.java:480)
	at reactor.core.publisher.Operators$MultiSubscriptionSubscriber.set(Operators.java:2367)
	at reactor.core.publisher.FluxConcatMapNoPrefetch$FluxConcatMapNoPrefetchSubscriber.onNext(FluxConcatMapNoPrefetch.java:202)
	at reactor.core.publisher.FluxBufferPredicate$BufferPredicateSubscriber.onNextNewBuffer(FluxBufferPredicate.java:317)
	at reactor.core.publisher.FluxBufferPredicate$BufferPredicateSubscriber.tryOnNext(FluxBufferPredicate.java:227)
	at reactor.core.publisher.FluxBufferPredicate$BufferPredicateSubscriber.onNext(FluxBufferPredicate.java:200)
	at reactor.core.publisher.FluxPeekFuseable$PeekFuseableConditionalSubscriber.onNext(FluxPeekFuseable.java:503)
	at reactor.core.publisher.FluxMapFuseable$MapFuseableConditionalSubscriber.onNext(FluxMapFuseable.java:299)
	at reactor.core.publisher.FluxContextWrite$ContextWriteSubscriber.onNext(FluxContextWrite.java:107)
	at reactor.core.publisher.FluxDoFinally$DoFinallySubscriber.onNext(FluxDoFinally.java:113)
	at reactor.core.publisher.FluxConcatArray$ConcatArraySubscriber.onNext(FluxConcatArray.java:180)
	at reactor.core.publisher.FluxFlattenIterable$FlattenIterableSubscriber.drainAsync(FluxFlattenIterable.java:453)
	at reactor.core.publisher.FluxFlattenIterable$FlattenIterableSubscriber.drain(FluxFlattenIterable.java:724)
	at reactor.core.publisher.FluxFlattenIterable$FlattenIterableSubscriber.onNext(FluxFlattenIterable.java:256)
	at reactor.core.publisher.FluxMap$MapSubscriber.onNext(FluxMap.java:122)
	at reactor.core.publisher.FluxPeek$PeekSubscriber.onNext(FluxPeek.java:200)
	at reactor.core.publisher.FluxMap$MapSubscriber.onNext(FluxMap.java:122)
	at reactor.netty.channel.FluxReceive.drainReceiver(FluxReceive.java:294)
	at reactor.netty.channel.FluxReceive.onInboundNext(FluxReceive.java:403)
	at reactor.netty.channel.ChannelOperations.onInboundNext(ChannelOperations.java:426)

@@ -75,11 +76,12 @@ void functionCallTest() {

@Test
void streamFunctionCallTest() {
Copy link
Member

Choose a reason for hiding this comment

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

same test failure as the other streaming tests

Copy link
Contributor Author

Choose a reason for hiding this comment

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

same test failure as the other streaming tests

hi, @markpollack i checked failed tests, sometimes, tool call might get split. i fixed this issue on PR #1276

Copy link
Member

Choose a reason for hiding this comment

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

thank you!

@markpollack
Copy link
Member

some issues in streaming, but as with the other PRs you made, it is better to get this in as it is a big step forward.

Thanks and apologies for the delay.

merged in 0927bd1

@markpollack markpollack added this to the 1.0.0-M2 milestone Aug 23, 2024
@mxsl-gr
Copy link
Contributor Author

mxsl-gr commented Aug 26, 2024

some issues in streaming, but as with the other PRs you made, it is better to get this in as it is a big step forward.

Thanks and apologies for the delay.

merged in 0927bd1

thanks for you merged my PR, when i committed this PR, all tests passed. i will check it out later to see what the problem is

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants