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 calculate_short_bonds_given_deposit #199

Merged
merged 4 commits into from
Oct 4, 2024

Conversation

dpaiton
Copy link
Member

@dpaiton dpaiton commented Oct 2, 2024

Resolved Issues

working towards #185
Also makes more progress towards #21

Description

This provides an alternative function to calculate_max_short that includes a tolerance parameter and guarantees* convergence with sufficient number of iterations.

I changed the signature & behavior from calculate_max_short in these ways:

  • Most importantly, we now ask for the absolute max short (aka pool's max regardless of budget) as an input argument. This is to make clear to the user that there are two required iterative processes to get this answer: first one is to find the absolute max short, second is to find a max short relative to a budget.
  • This function should also not be used with an unlimited budget to find the max short. Use calculate_absolute_max_short for that. If you provide a budget that is larger than the max possible, then this function will throw an error.
  • The result is guaranteed to be less than or equal to your budget; it will not overshoot the target base amount even if the result is within tolerance and solvent.

*I tested a lot of options, but I don't have any formal guarantees on convergence. The most extreme values I tried was 1e5 tolerance with 1M steps. These two knobs can be tuned as needed for trading off compute speed against tolerance.

Remaining tasks for future PRs

  • The new test is pretty janky because of the need to put several checks around calculating the absolute max bond amount. That function needs to be investigated next.

  • I still need to make the wasm & python bindings. This should be done after I fix up absolute max bond amount.

  • I created this as a new function because I want to give it a chance to exist on its own and be tested before we deprecate the existing calculate_max_short function. Once we have battle tested it, we will want to do a cleanup.

  • The method for calculating the conservative price is still not guaranteed to be safe.

Copy link
Contributor

@jrhea jrhea left a comment

Choose a reason for hiding this comment

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

Great work!

@dpaiton dpaiton force-pushed the dpaiton/iterative-approx-bonds branch from b762857 to f2cbac5 Compare October 4, 2024 16:41
@dpaiton dpaiton enabled auto-merge (squash) October 4, 2024 17:13
@dpaiton dpaiton merged commit def72bb into main Oct 4, 2024
8 checks passed
@dpaiton dpaiton deleted the dpaiton/iterative-approx-bonds branch October 4, 2024 17:21
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