-
-
Notifications
You must be signed in to change notification settings - Fork 5.6k
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
revert linspace to creating a vector #18492
Comments
We already have several spellings for getting the |
That was originally my thinking as well, but currently we don't have an easy spelling for getting linspace result as a vector with always correct end-points. |
I.e. #14420. @timholy's suggestion is certainly appealingly simple, but it really whiffs on a huge number of cases for internal points. I really don't think it makes sense to have this type and have it return half-baked results. Especially when the FloatRange behavior would actually be what one wants in those same cases. |
I disagree. If you care about the steps looking right, use the syntax that specifies a step. If you care about the number of elements and final endpoint, use the syntax that specifies those. They're different algorithms. With different spellings. The only thing I'd change about Tim's implementation is how it prints itself; it should look different from a FloatRange, because it is. Let's keep it as a lazy type. Folks are going to get more accustomed to all the specialized array types as we start using them more. And support for BLAS-incompatible array types is only getting better. |
export |
I think my brain spends 10% of its cycles running Jokes aside, I do think there's a legitimate issue of whether it's really worth calling such differences "broken." As you yourself have pointed out, there are limits to representing mathematical quantities with a finite number of bits. |
IEEE goes to great lengths to specify that addition, multiplication, and a few other operations need to be calculated correctly up to the last bit. With
|
... and for that you need do use the division operator. |
Referencing #17408 (comment), this version seems likely to get it essentially perfect: function Base.getindex(r::NewLinSpace, i::Integer)
d = r.len-1
s1 = fraction(r.len-i, r.leninv, d)
s2 = fraction(i-1, r.leninv, d)
s1*r.start + s2*r.stop
end
@inline function fraction(k, dinv, d)
x = k*dinv
x + (k-x*d)*dinv
end The only problem is it's 50% slower than our current LinSpace on a sum-over-all entries benchmark (eliding boundschecks). Perhaps there's something clever we could do to speed this up? |
Closed in favor of #18777. |
See #14420 and various other issues. It seems best to remove the LinSpace type from Base and move it to a package, keeping the
linspace
function but having it just return a vector of values.The text was updated successfully, but these errors were encountered: