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

vmod_std: Add std.pow #4001

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Conversation

fwsGonzo
Copy link
Contributor

@fwsGonzo fwsGonzo commented Oct 17, 2023

This adds std.pow.

Question: Link against libm?

@nigoroll
Copy link
Member

I am not against, but my question is: Do we really want to continue adding functions piecemeal or should we try to get something like libm covered as a whole?

@fwsGonzo
Copy link
Contributor Author

fwsGonzo commented Oct 18, 2023

I can add more functions, but I don't think it makes sense to add all of them. So, what about a list of functions that are most valuable? And I'll have some time next week to add them.

@nigoroll
Copy link
Member

I can not provide a list of "most valuable" functions. You need a function when you need it. Standard libraries provide functions which were deemed valuable before we knew they were, which is why I am asking if we should go for full coverage. Because we do not have complex numbers in VCL, I think this could be a starting point: https://sourceware.org/newlib/libm.html#Math an
While being at it, I have no experience with Floaring Point Environments. Do we need them?

@hermunn
Copy link
Member

hermunn commented Oct 18, 2023

I would prefer to have a separate VMOD for math.h functions, maybe called VMOD math.

Starting off with a few functions makes sense to me, for example,

math.pow(a, b)
math.exp(x)
math.sin(x)
math.cos(x)
math.abs(x)

but adding many should be very simple, too.

IMHO, the VMOD math functions should, by definition or convention, work on floating point, so there is no point with using names like fabs.

Furthermore, I think that also math.min(a, b) and math.max(a, b) are good functions to add, as they are hard to do in VCL.

For math.sqrt(x), where negative numbers give an error, I would prefer to return zero, and then the VCL writer will have to call math.error() to get 1 if there was an error in an argument, and math.error_message() to get the error as a STRING.

@asadsa92
Copy link
Contributor

I agree, this would mimic more or less what Python 3 has: https://docs.python.org/3/library/math.html
This would be familiar to the users.

@nigoroll
Copy link
Member

nigoroll commented Dec 4, 2023

bugwash is asking for examples of use cases

@bsdphk
Copy link
Contributor

bsdphk commented Dec 4, 2023

I do not feel any of this clears Gettys' first law:

»Do not add new functionality unless an implementor cannot complete
a real application without it.«

Convince me...

@hermunn
Copy link
Member

hermunn commented Dec 4, 2023

@fwsGonzo and me discussed this a while ago, and we agreed that maybe an open source VMOD math is the answer, and that we can consider moving it to Varnish Cache later on, if people are interested. I think we can close this ticket, and open a PR when VMOD math has gotten some wind under its wings.

@vaygr
Copy link

vaygr commented Dec 4, 2023

@hermunn closing this issue without some initial movement on the math module would hide any potential need for this raised here.

My use-case is quite trivial: exponential rate-limiting for IPs with data stored in Redis, that's used like so:

db.command("SET");
…
db.push(std.real2integer(120.0 * std.pow(2.0, std.real(req.http.seen, 1.0) - 1.0), 1));

An alternative was to use built-in support for ^ in Lua, but it's not portable should another storage be used or Lua support is not available:

db.command("EVAL");
db.push("return 120 * 2 ^ (" + req.http.seen + " - 1)");
…

@nigoroll
Copy link
Member

nigoroll commented Dec 4, 2023

Interesting. Without any information on the background at all, I did wildly guess during bugwash:

(15:45:26) slink: predictive rate limiting comes to my mind at least

Some library functions are so fundamental, that I think we should just make them available fully:

(15:44:42) slink: I think the main argument for aiming for full libX coverage is that it's cumbersome to PR every time you notice you need something. I do not understand why hermunn says he sees a case for trig functions, but hey, what do I know about his VCL?

@dridi
Copy link
Member

dridi commented Dec 5, 2023

During bugwash I suggested borrowing the ** operator from python to make this less cumbersome:

-db.push(std.real2integer(120.0 * std.pow(2.0, std.real(req.http.seen, 1.0) - 1.0), 1));
+db.push(std.real2integer(120.0 * (2.0 ** (std.real(req.http.seen, 1.0) - 1.0)), 1));

But only slightly it would seem.

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.

None yet

7 participants