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

Replace log2 and log10 with Julia ports. #27137

Closed
wants to merge 6 commits into from
Closed

Conversation

pkofod
Copy link
Contributor

@pkofod pkofod commented May 17, 2018

  • fix license
  • add tests
  • consider moving all sorts of helper functions (highword, zero_out_lowword) up in the file tree so they're not scattered around in various files.

@pkofod pkofod mentioned this pull request May 17, 2018
17 tasks
_log1p_t2(w::Float32) = @horner(w, lg1, lg3)
_log1p_t2(w::Float64) = @horner(w, Lg1, Lg3, Lg5, Lg7)
"""
_log1p(f::Union{Float32, Float64})
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This docstring mentions _log1p but the method is called k_log1p.

@simonbyrne
Copy link
Contributor

simonbyrne commented May 23, 2018

I've tweaked this a bit to use generic interface for arbitrary-base logs.

This should provide a good base for improving the accuracy of arbitrary bases (#18298). Combined with the recent optimiser changes, we could make things like
log(1000,x) as accurate and fast as the "native" logs.

@ViralBShah
Copy link
Member

Bump - can we get this in?

@pkofod
Copy link
Contributor Author

pkofod commented Jul 17, 2018

I’m not in possession of a computer I can dev on until next week

@ViralBShah
Copy link
Member

Bump - in order to get this (and the rest of the libm) things merged, in case we are close enough.

@ViralBShah
Copy link
Member

Bump - would be nice to revive this.

@pkofod
Copy link
Contributor Author

pkofod commented Dec 17, 2018

If the kids cooperate (by falling asleep instead of screaming for random things), I have some time tonight to add the benchmarks and tests.

@pkofod
Copy link
Contributor Author

pkofod commented Dec 17, 2018

@simonbyrne I'm off to bed. Did some benchmarking, and it seems fine (faster even?) but I havn't rebased yet, so I need to make sure it's not because of that. If you have time to run some benchmarks, I'd appreciate it.Also curious if there are any specific values you'd prefer to test against.

@pkofod
Copy link
Contributor Author

pkofod commented Dec 18, 2018

Seems to run a bit faster but I only checked it against 1.0.2 prebuilt binaries, but I did a stupid thing and my system is now building from scratch. >_>

base/special/log.jl Outdated Show resolved Hide resolved
@ViralBShah
Copy link
Member

What do we need to get this merged?

@pkofod
Copy link
Contributor Author

pkofod commented May 16, 2019

I've not sure if I ever got around to rebuilding as mentioned above, but given some recent oddities benchmarking these things, I think we should benchmark them once more, and I'd love to have some knowledgeable people looking over my shoulder to make sure that there are no regressions.

# is preserved.
# ====================================================

# Constants and utilities
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is this an orphan comment that could be removed?

Copy link

@aminya aminya left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tested this, and it works. Is there a reason that we have not merged it?
My test branch:
https://github.com/aminya/julia/tree/log2log10

@StefanKarpinski
Copy link
Member

Bump, would be lovely to get this out of WIP...

@pkofod pkofod changed the title [WIP] Replace log2 and log10 with Julia ports. Replace log2 and log10 with Julia ports. Dec 3, 2019
@pkofod
Copy link
Contributor Author

pkofod commented Dec 3, 2019

WIP is probably also no longer descriptive... I think the missing step here is proper benchmarking.
Edit: but I might want to install a second OS on my computer so I can build Julia again (I'm not doing that on windows...)

@pkofod
Copy link
Contributor Author

pkofod commented Jan 31, 2020

what have i done 😱

edit : went back to my old commit

@pkofod pkofod closed this Jan 31, 2020
@pkofod pkofod reopened this Jan 31, 2020
@ViralBShah
Copy link
Member

I am wondering that if we are redoing this, do we prefer getting the Tang algorithm that is in libm.jl?

@pkofod
Copy link
Contributor Author

pkofod commented Feb 29, 2020

I am wondering that if we are redoing this, do we prefer getting the Tang algorithm that is in libm.jl?

Good question. Since this is essentially sure, I'm not sure it's worth it. I mean we can always try that one after. The main thing to be done here is to test for regressions, but I've not felt confident doing benchmarks on these types of functions in Julia for a while. It's so hard to know if things get inlined/constantfolded/etc.

@oscardssmith
Copy link
Member

#39556 supersedes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
maths Mathematical functions
Projects
None yet
Development

Successfully merging this pull request may close these issues.

10 participants