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

fix internal precision loss #10513

Merged
merged 6 commits into from Sep 23, 2020
Merged

Conversation

bryevdv
Copy link
Member

@bryevdv bryevdv commented Sep 22, 2020

I could not find any other CenterRotatable that needed similar treatment, and I verified that hover works with just these changes.

@mattpap any thoughts welcome:

  • is it worth trying to test the array type to use NumberArray when possible instead of Array for everything in these places?
  • Ideas for tests? I spent ~30 minutes trying to get a Rect._map_data unit test that worked but did not succeed
 #!/bin/env python
from datetime import datetime, timedelta
import numpy as np
from bokeh.plotting import figure, show
from bokeh.models import DatetimeTickFormatter

## The line below is the key -- If I subtract 50 years from the current date, the rectangles appear just fine.
## If i don't subtract them, the rectangles are not rendered.
t0 = datetime(2020, 9, 21, 23, 22, 25, 624793) # datetime.utcnow() #- timedelta(weeks=52 * 50)

times = t0.timestamp() * 1e3 + np.linspace(0, 5, 100, endpoint=False) * 3600 * 1e3
times = times.astype('datetime64[ms]')

plot = figure(
      output_backend='svg',
      plot_height=500,
      plot_width=1280,
      x_axis_label='UTC Time',
      y_axis_label="",
      y_axis_type='linear',
      tooltips="$index"
)

plot.xaxis.formatter = DatetimeTickFormatter(
      hours=['%H:%M'],
      minutes=['%H:%M'],
      seconds=['%H:%M']
)

dm = np.linspace(0, 100, 1)
width = 100000 # (np.diff(times)[0] * 0.8).astype(timedelta)
times, dm = np.meshgrid(times, dm)
plot.rect(
      x=list(times.astype(datetime).flatten()),
      y=dm.flatten(),
      width=width, height=100 / 20, name='rect',
      line_color="red", alpha=0.6
)

show(plot)

Screen Shot 2020-09-22 at 2 33 43 PM

@mattpap
Copy link
Contributor

mattpap commented Sep 22, 2020

Given the localized and private nature of those changes, I suppose this is a fine workaround, but in the long term we need to use the right word size for the data, even if it means that we will need to use synthetic coordinates and/or use more memory in webgl backend.

@bryevdv
Copy link
Member Author

bryevdv commented Sep 22, 2020

@mattpap this has nothing to do with webgl, as we discussed previously, we are doing in all the mapping in non-webgl now, which means that webgl only sees things that fit in float32 (e.g. screen coords). More pointedly, this problem surfaced on non-webgl backends. The issue is specifically wrt to any 64-bit input data e.g timestamps, and their conversion to screen coordinates, which needs to be handled carefully enough to not lose precision internally along the way. I am happy to consider any general breaking changes for this for 3.0 (that is what #10379 is for hashing out).

@bryevdv bryevdv force-pushed the bryanv/10488_rect_datetime_precision branch from 226976b to 64a4ca4 Compare September 22, 2020 23:44
@bryevdv
Copy link
Member Author

bryevdv commented Sep 22, 2020

@mattpap It think the BokehJS-CI tests just need update, the actual code from the issue in question looks correct:

Screen Shot 2020-09-22 at 4 49 49 PM

i.e. probably a bunch of fractional pixel diffs.

@bryevdv
Copy link
Member Author

bryevdv commented Sep 22, 2020

Marked as ready. Happy to add a test but would need some guidance.

@bryevdv bryevdv added this to the 2.3 milestone Sep 23, 2020
@bryevdv bryevdv merged commit 4dfca93 into branch-2.3 Sep 23, 2020
@bryevdv bryevdv deleted the bryanv/10488_rect_datetime_precision branch September 23, 2020 18:16
poldavezac pushed a commit to poldavezac/bokeh that referenced this pull request Sep 26, 2020
* fix internal precision loss

* use Float64Array internally

* update integration tests images

* regression test

* lint

* remaining baselines
This was referenced Oct 14, 2020
@mattpap mattpap modified the milestones: 2.3, 2.2.3 Oct 16, 2020
mattpap pushed a commit that referenced this pull request Oct 16, 2020
* fix internal precision loss

* use Float64Array internally

* update integration tests images

* regression test

* lint

* remaining baselines
bryevdv added a commit that referenced this pull request Oct 19, 2020
* Backport: fix internal precision loss (#10513)

* fix internal precision loss

* use Float64Array internally

* update integration tests images

* regression test

* lint

* remaining baselines

* Backport: Pin mypy (part of #10568)

* Add 2.2.3 release notes

* Add 2.2.3 to versions.json

Co-authored-by: Bryan Van de Ven <bryan@bokeh.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

In python3, rectangle does not appear when x axis is type datetime
2 participants