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

Use safe default values #1148

Open
RoyKas opened this issue Aug 30, 2023 · 1 comment
Open

Use safe default values #1148

RoyKas opened this issue Aug 30, 2023 · 1 comment

Comments

@RoyKas
Copy link

RoyKas commented Aug 30, 2023

In the leastexpensive.py market strategy defaults for the maximum pricing are "Inf". If requestors omit to set sensible limits, they pay the price. Since it takes some time to gather offers, the market strategy is likely to select the first provide it finds, even if it has absurd high pricing.
Why not set a safe default value like 0, to prevent unnecessary loss of GLM and annoying the few requestors we have. This will force them to set the limit, and if they don't care, set the limit to "Inf" themselves.

@krunch3r76
Copy link
Contributor

krunch3r76 commented Sep 18, 2023

thank you for raising this concern!

it's essential to note that the responsibility lies with the requestor app developer to provide mechanisms to control pricing. however, the examples we see are primarily designed for demonstration purposes on the testnet. setting to 0 would increase code complexity in all the testnet examples and might make onboarding a little more cumbersome for people writing short scripts to learn with.

in my perspective, the default strategy serves as a placeholder or reminder. it nudges requestor app developers to set or facilitate the setting of logical values.

but i agree, more than a nudge can be desirable. the simplest way to do what you ask, force the values to be set, is to have yapapi offer a utility function that takes as arguments the max pricing and returns an instance of LeastExpensiveLinearPayuMS according to these inputs. in the very least, setting strategy no longer is an optional argument and tweaking becomes straightforward, satisfying your requirement to force settings while still allowing for defaults to be arbitrarily high.

because without such a helper you can see how tedious it can be, starting with the constructor on LeastExpensiveLinearPayuMS:

def __init__(
    self,
    expected_time_secs: int = 60,
    max_fixed_price: Decimal = Decimal("inf"),
    max_price_for: Mapping[Union[Counter, str], Decimal] = MappingProxyType({}),
):

here, there's no explicit limit set, aside from the expected_time_secs as you observed. (there's a separate discussion on the arbitrariness of this parameter in relation to thread count #1147).

and to tailor the strategy, a requestor app developer would indeed need to dive into the source code to match the strings corresponding to the offer keys. this is how it looks:

strategy = LeastExpensiveLinearPayuMS(
    max_price_for={
        com.Counter.CPU.value: Decimal("0.2"),
        com.Counter.TIME.value: Decimal("0.1"),
        "golem.usage.custom.counter": Decimal("0.1"),
    }
)

in this case, the enum values defined in the cpu.Counter are looked up to access the string keys. additionally, there's a custom field from yapapi custom_usage_counter.py. as for the max_fixed_price, it defaults to infinity, but as you rightly pointed out, a sensible limit should be set by the requestor.

in conclusion, the defaults currently serve as gentle reminders, and it's up to developers to tailor these strategies to their specific requirements.

yet it is not clear to beginner developers how this is done. it takes some digging! the docs(trings) could definitely be enhanced to make it easier for newbie developers to facilitate setting of sane defaults.

on the other hand, considering the challenges beginners face, there's merit in your suggestion to set the default to 0 when not on the testnet and perhaps someone can propose the logic for that.

an alternative but heavier general one size fits all implementation could involve hardcoding the maximum pricing, drawing inspiration from how yapapi.payload.repo.vm handles min_mem_gib, min_storage_gib, and min_cpu_threads. however, instead of placing this on the repo.vm object, it would be more logical to position it within the Golem object's constructor, alongside the strategy.

yet, there's a catch: allowing the strategy to override these settings might not be in sync with the current API design. it would potentially necessitate the strategy object to refer back to the Golem object, which could introduce intricacies, especially considering Golem's modular design paradigm.

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

No branches or pull requests

2 participants