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

Test price move #142

Merged
merged 56 commits into from
Sep 15, 2023
Merged

Test price move #142

merged 56 commits into from
Sep 15, 2023

Conversation

jepidoptera
Copy link
Contributor

Two new notebooks here looking at what happens when an LP invests or withdraws at the height of a temporary price move.

@jepidoptera jepidoptera requested a review from poliwop June 29, 2023 17:17
Copy link
Collaborator

@poliwop poliwop left a comment

Choose a reason for hiding this comment

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

Looks correct. I would suggest extending it as follows, probably in new notebooks initially (but they maybe can replace the existing notebooks once they work):

There are 3 variables that may impact the amount LRNA value changes that I'd like analyzed:

  • Size of the temp price move (this is studied in the existing notebook)
  • Size of the liquidity added/removed during the price move
  • Weight of the asset in pool to which liquidity is added

The existing notebooks pretty much only look at the connection between price move size and LRNA price movement, but all 3 of these matter, I suspect. The existing notebooks make their price moves over time, which is useful initially to see that things are working as expected, but for the new notebooks I think is unnecessary.

I'd suggest that for each variable above, fix the other 2 at something reasonable and just vary the 3rd, and graph that.

  • Vary size of liquidity add from 1% to 50%
  • Vary size of price move from -90% to 10x (same as current notebooks)
  • Vary initial weight of asset from 5% to 80% (which simulates correlated price movement, actually)

hydradx/notebooks/Omnipool/TestPriceMoveLP.ipynb Outdated Show resolved Hide resolved
hydradx/notebooks/Omnipool/TestPriceMoveLP.ipynb Outdated Show resolved Hide resolved
hydradx/notebooks/Omnipool/TestPriceMoveLP.ipynb Outdated Show resolved Hide resolved
hydradx/notebooks/Omnipool/TestPriceMoveWithdraw.ipynb Outdated Show resolved Hide resolved
hydradx/notebooks/Omnipool/TestPriceMoveWithdraw.ipynb Outdated Show resolved Hide resolved
hydradx/notebooks/Omnipool/TestPriceMoveWithdraw.ipynb Outdated Show resolved Hide resolved
hydradx/notebooks/Omnipool/TestPriceMoveLP.ipynb Outdated Show resolved Hide resolved
# Conflicts:
#	hydradx/tests/test_omnipool_amm.py
@jepidoptera jepidoptera requested a review from poliwop July 14, 2023 22:00
Copy link
Collaborator

@poliwop poliwop left a comment

Choose a reason for hiding this comment

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

It looks like there are still outstanding issues from the last review in TestPriceMoveLP.ipynb? Broadly, TestPriceMoveWithdraw.ipynb now looks pretty good.

hydradx/notebooks/Omnipool/TestPriceMoveWithdraw.ipynb Outdated Show resolved Hide resolved
" [1 / (ticks_number - 1) * i * len(events) for i in range(ticks_number)],\n",
" [f'{int(i * 100)}%' for i in tkn_weights[::10]]\n",
" )\n",
" plt.title(f\"price move: {price_moves[pump_factor]}\")\n",
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why does this graph look like a sawtooth graph instead of a smooth one? Doesn't make sense to me...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It has to do with the optimization in omnipool_arbitrage that we discussed, specifically the one where the arbitrageur skips certain assets if their price differential is too small to make a profit. Since that check is performed at the beginning of the function, and subsequent trades during the function can cause prices to change, this results in some trades being skipped that would have become profitable, ultimately messing up the graph. My for-now solution was to comment out that code block, re-run the notebook, and push the result with the better graph, but I'd suggest adding a way to disable that optimization in the future (maybe skip it if arb_precision > 1?)

Copy link
Collaborator

Choose a reason for hiding this comment

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

This isn't the solution that's currently checked in, is it?

hydradx/model/amm/trade_strategies.py Outdated Show resolved Hide resolved
Copy link
Collaborator

@poliwop poliwop left a comment

Choose a reason for hiding this comment

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

This looks good, as long as the notebook checked in reflects the code run... see my question.

# Conflicts:
#	hydradx/model/amm/trade_strategies.py
#	hydradx/tests/test_omnipool_amm.py
@jepidoptera jepidoptera merged commit d5d565f into main Sep 15, 2023
2 checks passed
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