Change surplus based tests to use new data from orderbook #91
Conversation
This file contains 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
fhenneke
changed the title
1) get bare response from orderbook api for a given uid 2) use order data from api and execution data from competition endpoint to create a Trade object
- uses new orderbook methods to obtain order data - execution data is read from competition data. before it was read from calldata. the latter is not possible anymore for non-winning solutions. - a check on fees being smaller than costs was removed. this might introduce a bit of noise. but the data on costs is not available with the new data format. - `trade_alt_dict` was renamed to `trade_alt_list` and the type was changed from a dict to a list. this allows for the case of solvers submitting multiple solutions. - removes dependence on web3 api. that api was used before for simple access to order information. since it is not easy to link that to uids and since the ordering in the competition data changed compared to the ordering in the call data, it became easiest to just query the order endpoint. -
still need to find a better example settlement
it is used in two tests and only depends on the orderbook api at the moment.
- use `get_uid_trader` from `orderbook_api` - remove dependency on web3api - add more type information, which is required due to some function returning `Type | None` now - remove using objective instead of surplus in case of small objective (the objective is not available from the competition endpoint anymore) - remove old tests which used old competition data format
which were not really used and resulted in linter error
fhenneke
force-pushed
the
new_competition_format
branch
from
289eb90 to
d47e8b2
Compare
|
Although I still haven't done a proper review, I am approving since I am the blocking factor here, and let's see if there are any issues when this starts running (in any case, it will not really break anything at the current state of things). |
harisang
approved these changes
Jan 16, 2024
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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.
This PR addresses #90 for the competition surplus test and combinatorial auction test.
The main changes are:
Before, information on trades (order data and order execution) were read from calldata. For non-winning solutions this is not possible anymore. For the winning solution it is still possible. But due to changes to the ordering of orders in the competition data, it was not easy for me to reconstruct the link from uid to trade. Thus, all information is not obtained through the orderbook api.
In principle, information on fees for non-winning solutions could be computed from clearing prices in combination with effective buy and sell amount. This is not done here since fees do not matter for surplus. Also clearing prices are not yet part of the data in the competition endpoint.
Information on costs for non-winning solutions is and will not available. A test of the form
fees >= costshad to be removed due to that.The old code had a "bug" where only one execution per solver was studied. This is fixed now and the test should work correctly with solvers submitting multiple solutions. The fix required changing the data format for alternative solutions from a
dictto alist