Skip to content
This repository has been archived by the owner on May 13, 2022. It is now read-only.

For CJs: ignore makers with overlapping offer ranges #710

Closed
wants to merge 2 commits into from

Conversation

AlexCato
Copy link
Contributor

support.py: checkes if makers have overlapping offers and removes them from being considered for joins

This is done efficiently in O(n) by adding up the ranges of a maker's offers.
Example:
Maker has 3 offers:
a) min=1, max=3
b) min=1.5, max=4
c) min=3, max=5

First, the "total" maximum range of that maker is highest_max - lowest_min (in this case: 5-1=4).
Now the single ranges are iteratively added up and checked against the "total" max.

If there is no overlap, then the total max will always be >= the added up ranges of the individual offers.

Currently, about 25% of all makers use overlapping ranges and would be filtered out by this.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.3%) to 76.988% when pulling 4ee0f39 on AlexCato:ignore_overlapping into 0c919d2 on JoinMarket-Org:develop.

@AdamISZ
Copy link
Member

AdamISZ commented Feb 12, 2017

Took another quick look at algo (not the code yet)

This does allow overlaps if entire range not covered, right? (example: (1,3), (7,9), (12,14), (13,15) -> total range 14, sum 8); are we saying therefore that since they're not exploiting the maximum range they could, it's OK? On reflection, that's probably not OK, since maybe the "action" is in the 12-15 range and they still get away with it by using the "dummy" low offers.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.3%) to 76.992% when pulling 5df803b on AlexCato:ignore_overlapping into 0c919d2 on JoinMarket-Org:develop.

@AlexCato
Copy link
Contributor Author

Agreed, there can still be overlaps.
I'd also prefer to solve this once and for all and just check for actual overlaps; combined with #711 there is also no more performance issues.

Will change the algorithm as soon as I have some more time on my hands.

@AlexCato
Copy link
Contributor Author

Will build this on top of #712 once merged and improve/change it as discussed.

@AlexCato AlexCato closed this Feb 24, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants