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

Add credit usage simulation #87

Merged
merged 13 commits into from
Feb 26, 2025
Merged

Add credit usage simulation #87

merged 13 commits into from
Feb 26, 2025

Conversation

davidotte
Copy link
Collaborator

Change Description

Try to be precise. You can additionally add comments to your PR, this might help the reviewer a lot.

Add credit usage simulation for TabPFN extensions. For usage just import with from tabpfn_client.mock_prediction import check_api_credits and add decorator @check_api_credits to a function. Then, this function will first be simulated (both the execution time and credit usage), it will be checked if the estimated credit usage exceeds the current credits of the user. If yes an error is raised, if not the function gets normally executed.

Few notes:

  • With adding a latency offset of 1s for each request, in my tests the simulated cost always overestimated the actual credit usage a bit, so this seems to work quite well. However, I don't know if other users will experience a significantly lower latency.
  • For using the decorator in an extension, we probably have to create a common decorator for both client and tabpfn that simply does nothing if tabpfn-client is not used.
  • I slightly changed the get_api_usage function Anshul implemented, such that the string for user info is created at config level and not at client level. This allows me to directly access these values.

If you used new dependencies: Did you add them to requirements.txt?

Who did you ping on Mattermost to review your PR? Please ping that person again whenever you are ready for another review.

@noahho

Breaking changes

If you made any breaking changes, please update the version number.
Breaking changes are totally fine, we just need to make sure to keep the users informed and the server in sync.

Does this PR break the API? If so, what is the corresponding server commit?

no

Does this PR break the user interface? If so, why?

no


Please do not mark comments/conversations as resolved unless you are the assigned reviewer. This helps maintain clarity during the review process.

@davidotte davidotte requested a review from noahho February 17, 2025 11:44
Copy link
Collaborator

@noahho noahho left a comment

Choose a reason for hiding this comment

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

Great, this looks like a nice solution! Please add some more comments and take a look at my suggestions

@davidotte davidotte force-pushed the add-credit-usage-simulation branch from d27aceb to 51c956f Compare February 19, 2025 12:16
Copy link
Collaborator

@noahho noahho left a comment

Choose a reason for hiding this comment

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

please only take a look at the one comment

@noahho
Copy link
Collaborator

noahho commented Feb 23, 2025

Great, LGTM!

@davidotte davidotte merged commit 355cdb0 into main Feb 26, 2025
5 checks passed
@davidotte davidotte deleted the add-credit-usage-simulation branch February 26, 2025 09:42
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.

2 participants