-
-
Notifications
You must be signed in to change notification settings - Fork 31
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
More optimize_ticks improvements #92
base: master
Are you sure you want to change the base?
Conversation
db01b5e
to
5effa77
Compare
I rebased and formatted this PR just to better see the difference to the current version. It would still be neccessary to check whether the algorithms are doing what they shoud do. |
Is this really rebased ? It looks like it reverts recent additions (especially #136 which is critical). |
Well, when in doubt, I kept the original version, so there is certainly stuff that'd need to be adapted. |
I think this is as far as I can help here. |
Following #83 (comment), I realized that actual floating point are not really used during the optimization, so most FP calculations can be avoided. Representing the ticks as a new
Ticks
type and calculating FP values only ongetindex
makes things both simpler and faster.Also, unitful quantities work now.
This makes two tests fail. Specifically,
optimize_ticks(1.0,1.0+eps())
andoptimize_ticks(1.0-eps(),1.0)
, so I marked them as broken. These do issue a warning and the result is sensible, so I think it's fine.