-
Notifications
You must be signed in to change notification settings - Fork 117
Optimized stolarsky_mean
#2274
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
base: main
Are you sure you want to change the base?
Optimized stolarsky_mean
#2274
Changes from all commits
3a2037f
5c2b302
d8c1901
8040dbd
744a9dd
008bc8e
e317708
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
@@ -412,8 +412,15 @@ Given ε = 1.0e-4, we use the following algorithm. | |||||||||||||
c3 = convert(RealT, -1 / 21) * (2 * gamma * (gamma - 2) - 9) * c2 | ||||||||||||||
return 0.5f0 * (x + y) * @evalpoly(f2, 1, c1, c2, c3) | ||||||||||||||
else | ||||||||||||||
return (gamma - 1) / gamma * (y^gamma - x^gamma) / | ||||||||||||||
(y^(gamma - 1) - x^(gamma - 1)) | ||||||||||||||
if isinteger(gamma) | ||||||||||||||
yg = y^(gamma - 1) | ||||||||||||||
xg = x^(gamma - 1) | ||||||||||||||
else | ||||||||||||||
yg = exp((gamma - 1) * log(y)) # equivalent to y^(gamma - 1) but faster for non-integers | ||||||||||||||
xg = exp((gamma - 1) * log(x)) # equivalent to x^(gamma - 1) but faster for non-integers | ||||||||||||||
end | ||||||||||||||
return (gamma - 1) / gamma * (yg * y - xg * x) / | ||||||||||||||
(yg - xg) | ||||||||||||||
Comment on lines
+422
to
+423
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
Does this formatting work now? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. And does it make a difference if you avoid an additional division by rewriting this part as something like
Suggested change
? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It looks like there's not a big difference. It is slightly faster for reals, but slightly slower for integers.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I am mainly looking at the median results. There, the first option seems to be best all the time? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I agree with you, I will compare also full runs, to see if there are any noticeable differences. |
||||||||||||||
end | ||||||||||||||
end | ||||||||||||||
end # @muladd |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
isinteger
checks also the values of floating-point numbersThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That seems much faster, but I had to remove the specific parametrization. That would imply also to change the struct, so that, for instance,
2
can be accepted, instead of2.0
.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you please change the code
a few lines above to
and then check the
gamma isa Integer
option?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I get the following error when trying that. As far as I understood, that creates a loop because the main function is always defined for type Reals, and all the variables are bounded to be of the same type, hence the function calls itself and doesn't switch to the "main" one.
A workaround would be something like:
but again, shouldn't I allow the equation struct to accept different types and not promoting the
gamma
?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What I meant was basically the second option:
Using this approach,
gamma
can be anInteger
and will not be promoted.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Perfect, thank you! Sorry for the missunderstanding, I will introduce that.