Skip to content

max short fixes#137

Merged
dpaiton merged 21 commits intomainfrom
dpaiton/max-short-fixups
Jun 12, 2024
Merged

max short fixes#137
dpaiton merged 21 commits intomainfrom
dpaiton/max-short-fixups

Conversation

@dpaiton
Copy link
Contributor

@dpaiton dpaiton commented Jun 12, 2024

Resolved Issues

Partially addresses #121 & #136

Description

These changes arose in my other pr (#116) while trying to debug issues and high tolerances in short::max. There are a lot of commits in order to separate out meaningful changes from trivial ones. A majority of the changes are code reorganization, docstring fixes, and some additional sanity checks.

Major changes were:

  • fuzz_calculate_max_short

    • was renamed to more accurately reflect what it is testing -- max short in the budget constrained regime
    • was rewritten to ensure the budget is constrained. I believe the reason the failures were "intermittent" before was because it was not always hitting the failure case. It hits it a lot now.
    • the tolerance was updated so that it consistently passes, to give us an idea of how much it is off from what we want.
  • calculate_max_short
    this code was incorrect:

         let absolute_max_deposit =
           match self.calculate_open_short(max_bond_amount, open_vault_share_price) {
               Ok(d) => d,
               Err(_) => return Ok(max_bond_amount),

we do not want to return the bond amount if calculate_open_short is throwing an error. I updated it.

I ran the tests locally with FUZZ_RUNS=1_000 and FAST_FUZZ_RUNS=50_000 without any tests failing.

@dpaiton dpaiton marked this pull request as ready for review June 12, 2024 00:27
@dpaiton dpaiton changed the title max short fixups max short fixes Jun 12, 2024
Co-authored-by: Alex Towle <jalextowle@gmail.com>
dpaiton and others added 4 commits June 11, 2024 21:17
Co-authored-by: Alex Towle <jalextowle@gmail.com>
Co-authored-by: Alex Towle <jalextowle@gmail.com>
Copy link
Contributor

@jalextowle jalextowle left a comment

Choose a reason for hiding this comment

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

LGTM

@dpaiton dpaiton merged commit ce169d6 into main Jun 12, 2024
@dpaiton dpaiton deleted the dpaiton/max-short-fixups branch June 12, 2024 22:27
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