-
Notifications
You must be signed in to change notification settings - Fork 8
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
[Orchestration Convenience] Typed model parameters #180
[Orchestration Convenience] Typed model parameters #180
Conversation
orchestration/src/main/java/com/sap/ai/sdk/orchestration/OrchestrationAiModelParameters.java
Outdated
Show resolved
Hide resolved
orchestration/src/main/java/com/sap/ai/sdk/orchestration/OrchestrationAiModelParameters.java
Outdated
Show resolved
Hide resolved
orchestration/src/main/java/com/sap/ai/sdk/orchestration/OrchestrationAiModelParameters.java
Outdated
Show resolved
Hide resolved
orchestration/src/main/java/com/sap/ai/sdk/orchestration/OrchestrationAiModel.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As discussed in todays (after) daily..
- We want to offer API (via fields, methods or classes) for the available parameter keys.
- We can't reasonably maintain the full mapping of LLM provider / model parameters.
- We can carefully(?) assume a very small subset of keys to be available for all LLM providers.
How to translate that to code, needs to be aligned still.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The API looks good!
Very helpful.
orchestration/src/main/java/com/sap/ai/sdk/orchestration/OrchestrationAiModel.java
Outdated
Show resolved
Hide resolved
orchestration/src/main/java/com/sap/ai/sdk/orchestration/OrchestrationAiModel.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Otherwise LGTM. Maybe we can add the link to https://help.sap.com/docs/sap-ai-core/sap-ai-core-service-guide/harmonized-api somewhere in the Javadoc / sample code / docs
…model-parameters' into orchestration-convenience/typed-model-parameters
…ence/typed-model-parameters # Conflicts: # orchestration/src/main/java/com/sap/ai/sdk/orchestration/OrchestrationAiModel.java
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Would be helpful for demo purposes and general usability.