-
Notifications
You must be signed in to change notification settings - Fork 159
Adds information about the importance of adaptive allocations #1454
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
Merged
Merged
Changes from 9 commits
Commits
Show all changes
13 commits
Select commit
Hold shift + click to select a range
1573b58
Adds information about the importance of adaptive allocations
kosabogi b142d84
Fixes links
kosabogi df9f37c
Update explore-analyze/elastic-inference/inference-api.md
kosabogi 3acb326
Applies suggestions
kosabogi b5b0340
Additional information
kosabogi d0fdead
Applying suggestions
kosabogi 05a9a12
Update explore-analyze/elastic-inference/inference-api.md
kosabogi 4112605
Modifications based on feedback
kosabogi 92116f2
Merge branch 'main' into inference-adaptive-allocations
kosabogi d4edf6a
Applying suggestions
kosabogi 412e86e
Merge branch 'main' into inference-adaptive-allocations
kosabogi 7f64bc2
Merge branch 'main' into inference-adaptive-allocations
kosabogi f94ce67
Merge branch 'main' into inference-adaptive-allocations
kosabogi File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
If we're delegating to the trained model page, should this note still live here or would it make sense to move that too?
Uh oh!
There was an error while loading. Please reload this page.
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.
Where do you think it would make more sense to place it?
IMO it should live here because it links to the Trained model autoscaling page, where the adaptive allocations information is located. But maybe I'm misunderstanding your comment? 🤔
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.
::::{note}
goes into details about pricing implications but we've already sent readers to another page with "For more information about adaptive allocations and resources, refer to the trained model autoscaling documentation." So the flow feels wrong. I'd argue for moving the cost note before the link. And also perhaps it should beimportant
given it deals with real cost implications.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.
Sorry, I thought you were referring to the sentence before the note - but it totally makes sense now.
My point is that we should include it here because the support ticket that raised this issue were about customers using inference services without realizing the cost implications.
Part of this is already mentioned on the trained model autoscaling page, specifically:
In this case, I think the duplication is fine, as it's important to warn users about potential costs (according to the issue, this information was missing from this page.)
You're absolutely right that placing the note before that sentence interrupts the flow - I’ll move it and rework the wording a bit.
Thanks so much for your suggestions!