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

[Inference API] Use extractOptionalPositiveInteger instead of removeAsType in AzureAiStudioEmbeddingsServiceSettings #110366

Conversation

timgrein
Copy link
Contributor

@timgrein timgrein commented Jul 2, 2024

Same rationale as in #110278. Added explicit tests this time, will add them afterwards to the other two places I've already merged (I want to use randomNegativeInt also there, so waiting for this PR to merge first).

@timgrein timgrein added >non-issue :ml Machine learning Team:ML Meta label for the ML team v8.15.0 labels Jul 2, 2024
@elasticsearchmachine
Copy link
Collaborator

Pinging @elastic/ml-core (Team:ML)

@timgrein timgrein added v8.16.0 and removed v8.15.0 labels Jul 3, 2024
Copy link
Member

@davidkyle davidkyle left a comment

Choose a reason for hiding this comment

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

LGTM

* @return an <code>int</code> between <code>Integer.MIN_VALUE</code> and <code>-1</code> (inclusive) chosen uniformly at random.
*/
public static int randomNegativeInt() {
return randomInt() | Integer.MIN_VALUE;
Copy link
Member

Choose a reason for hiding this comment

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

Nice.

I was worried about the corner cases if randomInt returned 0 or MAX_VALUE but
0 | Integer.MIN_VALUE == Integer.MIN_VALUE
Integer.MAX_VALUE | Integer.MIN_VALUE == -1

@timgrein
Copy link
Contributor Author

timgrein commented Jul 4, 2024

@elasticmachine update branch

…xtract-optional-positive-integer-instead-of-remove-as-type
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
:ml Machine learning >non-issue Team:ML Meta label for the ML team v8.16.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants