Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Log reference solver (i.e., Quasimodo) solution when generating a ref_solver-surplus-ebbo error #70

Merged
merged 6 commits into from
Jul 20, 2023

Conversation

harisang
Copy link
Contributor

This PR adds logging of the solution generated by the reference solver, whenever this (in isolation) gives more surplus to an order that is executed by some solver in the competition.

@harisang harisang requested a review from fhenneke July 19, 2023 14:01
Copy link
Contributor

@fhenneke fhenneke left a comment

Choose a reason for hiding this comment

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

I would split the function for computing alternative trades into two.
Otherwise looks good.

@harisang harisang requested a review from fhenneke July 19, 2023 16:57
if trade_alt is None:
self.logger.warning(
f"No alternative trade for uid {uid} and "
ref_solver_response = self.solve_order_with_reference_solver(
Copy link
Contributor

Choose a reason for hiding this comment

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

Potentially rename this to solution_alt? But this might be confusing in the sense that the solution format is then different in solution and solution_alt. Otherwise we could also change the variable name in get_trade_alternative, which is currently solution.

Copy link
Contributor Author

@harisang harisang Jul 20, 2023

Choose a reason for hiding this comment

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

I would probably say now that the get_trade_alternative name for the function might be not very good after all. The function is oblivious to whether this is an alternative execution or not, and the only thing it does is parse a given execution and returns a trade.

- wrong function argument
- solver response is not modified anymore when extracting execution info
@fhenneke fhenneke self-requested a review July 20, 2023 09:40
@harisang harisang merged commit d288276 into main Jul 20, 2023
@harisang harisang deleted the log_quasimodo_solution branch July 20, 2023 16:29
@github-actions github-actions bot locked and limited conversation to collaborators Jul 20, 2023
# for free to subscribe to this conversation on GitHub. Already have an account? #.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants