From 6e6614401947fc79e1669484ac099391cb178074 Mon Sep 17 00:00:00 2001 From: Adam Mizerski Date: Fri, 8 Mar 2019 11:48:46 +0100 Subject: [PATCH 1/2] fix math of calculating scaled price for marketplace Related issues: #3453, #3479, #3964 --- golem/marketplace/__init__.py | 2 +- golem/marketplace/offerpool.py | 6 ++++++ golem/task/tasksession.py | 8 ++------ tests/golem/marketplace/test_offerpool.py | 10 +++++++++- 4 files changed, 18 insertions(+), 8 deletions(-) diff --git a/golem/marketplace/__init__.py b/golem/marketplace/__init__.py index 6ca4567bec..8942e42852 100644 --- a/golem/marketplace/__init__.py +++ b/golem/marketplace/__init__.py @@ -1 +1 @@ -from .offerpool import Offer, OfferPool # noqa pylint: disable=unused-import +from .offerpool import scale_price, Offer, OfferPool # noqa pylint: disable=unused-import diff --git a/golem/marketplace/offerpool.py b/golem/marketplace/offerpool.py index a3c033d0e8..1dcba85ee6 100644 --- a/golem/marketplace/offerpool.py +++ b/golem/marketplace/offerpool.py @@ -9,6 +9,12 @@ logger = logging.getLogger(__name__) +def scale_price(task_price: float, offered_price: float) -> float: + if offered_price == 0: + return float('inf') + return task_price / offered_price + + class Offer: def __init__( self, diff --git a/golem/task/tasksession.py b/golem/task/tasksession.py index d05911999f..37e3275ad0 100644 --- a/golem/task/tasksession.py +++ b/golem/task/tasksession.py @@ -20,7 +20,7 @@ from golem.core import variables from golem.docker.environment import DockerEnvironment from golem.docker.image import DockerImage -from golem.marketplace import Offer, OfferPool +from golem.marketplace import scale_price, Offer, OfferPool from golem.model import Actor from golem.network import history from golem.network.concent import helpers as concent_helpers @@ -623,11 +623,7 @@ def _offer_chosen(is_chosen: bool) -> None: task = self.task_manager.tasks[msg.task_id] offer = Offer( - scaled_price=( - msg.price / task.header.max_price - if task.header.max_price > 0 - else float('inf') - ), + scaled_price=scale_price(task.header.max_price, msg.price), reputation=get_provider_efficiency(self.key_id), quality=get_provider_efficacy(self.key_id).vector, ) diff --git a/tests/golem/marketplace/test_offerpool.py b/tests/golem/marketplace/test_offerpool.py index c9cfe038b6..a0ef8f036b 100644 --- a/tests/golem/marketplace/test_offerpool.py +++ b/tests/golem/marketplace/test_offerpool.py @@ -1,7 +1,15 @@ from unittest import TestCase from unittest.mock import Mock, patch, ANY -from golem.marketplace import Offer, OfferPool +from golem.marketplace import scale_price, Offer, OfferPool + + +class TestScalePrice(TestCase): + def test_basic(self): + assert scale_price(5, 2) == 2.5 + + def test_zero(self): + assert scale_price(5, 0) == float('inf') @patch('golem.marketplace.offerpool.task.deferLater') From 2c32d56e7619c51e8c3d2c916775722e267bab1e Mon Sep 17 00:00:00 2001 From: Adam Mizerski Date: Fri, 8 Mar 2019 13:29:46 +0100 Subject: [PATCH 2/2] test order_providers with extreme scaled_price --- golem/marketplace/offerpool.py | 4 +++- rust/golem/src/marketplace.rs | 16 ++++++++++++++-- tests/golem/marketplace/test_offerpool.py | 3 ++- tests/golem/marketplace/test_rust.py | 8 +++++--- 4 files changed, 24 insertions(+), 7 deletions(-) diff --git a/golem/marketplace/offerpool.py b/golem/marketplace/offerpool.py index 1dcba85ee6..738380ec4b 100644 --- a/golem/marketplace/offerpool.py +++ b/golem/marketplace/offerpool.py @@ -1,3 +1,4 @@ +import sys import logging from typing import List, Dict, ClassVar, Tuple @@ -11,7 +12,8 @@ def scale_price(task_price: float, offered_price: float) -> float: if offered_price == 0: - return float('inf') + # using float('inf') breaks math in order_providers, when alpha < 1 + return sys.float_info.max return task_price / offered_price diff --git a/rust/golem/src/marketplace.rs b/rust/golem/src/marketplace.rs index 699589bd54..330bc36c28 100644 --- a/rust/golem/src/marketplace.rs +++ b/rust/golem/src/marketplace.rs @@ -1,3 +1,5 @@ +use std::f64; + // Price sensitivity factor. 0 <= ALPHA <= 1 const ALPHA: f64 = 0.67; // Distrust factor. 1 <= D @@ -82,16 +84,26 @@ mod tests { r: 0.0, }, }, + Offer { + scaled_price: f64::MAX, + reputation: 15.0, + quality: Quality { + s: 0.0, + t: 0.0, + f: 0.0, + r: 0.0, + }, + }, ]; } #[test] fn order_providers_price_preference() { let offers = gen_offers(); - assert_eq!(order_providers_impl(offers, 1.0, PSI, D), vec![3, 1, 0, 2]); + assert_eq!(order_providers_impl(offers, 1.0, PSI, D), vec![4, 3, 1, 0, 2]); } #[test] fn order_providers_reputation_preference() { let offers = gen_offers(); - assert_eq!(order_providers_impl(offers, 0.0, PSI, D), vec![1, 2, 3, 0]); + assert_eq!(order_providers_impl(offers, 0.0, PSI, D), vec![1, 2, 4, 3, 0]); } } diff --git a/tests/golem/marketplace/test_offerpool.py b/tests/golem/marketplace/test_offerpool.py index a0ef8f036b..ff347b6d97 100644 --- a/tests/golem/marketplace/test_offerpool.py +++ b/tests/golem/marketplace/test_offerpool.py @@ -1,3 +1,4 @@ +import sys from unittest import TestCase from unittest.mock import Mock, patch, ANY @@ -9,7 +10,7 @@ def test_basic(self): assert scale_price(5, 2) == 2.5 def test_zero(self): - assert scale_price(5, 0) == float('inf') + assert scale_price(5, 0) == sys.float_info.max @patch('golem.marketplace.offerpool.task.deferLater') diff --git a/tests/golem/marketplace/test_rust.py b/tests/golem/marketplace/test_rust.py index 015f774d6b..4841ebac1a 100644 --- a/tests/golem/marketplace/test_rust.py +++ b/tests/golem/marketplace/test_rust.py @@ -1,4 +1,4 @@ -from golem.marketplace import Offer +from golem.marketplace import scale_price, Offer from golem.marketplace.rust import order_providers @@ -6,6 +6,8 @@ def test_order_providers(): offer0 = Offer(scaled_price=2., reputation=1., quality=(1., 1., 6., 1.)) offer1 = Offer(scaled_price=3., reputation=5., quality=(1., 3., 1., 4.)) offer2 = Offer(scaled_price=4., reputation=2., quality=(2., 1., 1., 3.)) - res = order_providers([offer0, offer1, offer2]) + offer3 = Offer(scaled_price=scale_price(5, 0), reputation=3., + quality=(1., 2., 3., 4.)) + res = order_providers([offer0, offer1, offer2, offer3]) # Actual order is not important, just that it is a permutation - assert sorted(res) == list(range(3)) + assert sorted(res) == list(range(4))