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

dief@t adds additional data point with wrong timestamp #5

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

prohde
Copy link
Contributor

@prohde prohde commented Jan 15, 2022

The implementation of dief@t adds an additional data point for each approach with the maximum time t, where t is either the parameter given or the maximum t of all approaches. This leads to wrong values of dief@t if an approach finishes earlier than the others.

Assuming the following data

approach answer time
a1 1 379956750.0
a1 ... ...
a1 2746 379966378.0
a2 1 61117396.0
a2 ... ...
a2 2746 61117396.0

Approach a1 is less efficient but is generates answers continuously while a2 is a blocking approach. The current implementation adds the data points (a1, 2746, 379966378.0) and (a2, 2746, 379966378.0). Hence, the area-under-the-curve and the value for dief@t for a2 is higher than the one for a1 which is clearly wrong. The value of dief@t for a2 should be 0.0. This PR provides the necessary changes to fix this issue.

@prohde
Copy link
Contributor Author

prohde commented Apr 8, 2022

I discovered that the additional data point is necessary in most cases. However, when there is a true blocking approach as in the example above, it breaks the intuition. So I added a switch to turn off the behavior. By default, the data point will be added.

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.

1 participant