Skip to content

fix internal precision loss#10513

Merged
bryevdv merged 6 commits intobranch-2.3from
bryanv/10488_rect_datetime_precision
Sep 23, 2020
Merged

fix internal precision loss#10513
bryevdv merged 6 commits intobranch-2.3from
bryanv/10488_rect_datetime_precision

Conversation

@bryevdv
Copy link
Copy Markdown
Member

@bryevdv bryevdv commented Sep 22, 2020 •
edited
Loading

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
mattpap reviewed Sep 22, 2020
View reviewed changes
@mattpap
Copy link
Copy Markdown
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
Copy Markdown
Member Author

bryevdv commented Sep 22, 2020 •
edited
Loading

@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
Copy Markdown
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
Copy Markdown
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
@bryevdv @depixusgenome
fix internal precision loss (bokeh#10513)
b3542e1 
* fix internal precision loss

* use Float64Array internally

* update integration tests images

* regression test

* lint

* remaining baselines
This was referenced Oct 14, 2020
Merged
Closed
@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
@bryevdv @mattpap
Backport: fix internal precision loss (#10513)
0eea02d 
* 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
@mattpap @bryevdv
Backports for 2.2.3 (#10586)
ea5c8fa 
* 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>
@bryevdv bryevdv mentioned this pull request Oct 23, 2020
Closed
@github-actions
Copy link
Copy Markdown

github-actions bot commented Oct 27, 2024

This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Oct 27, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@mattpap mattpap mattpap left review comments

Assignees

No one assigned

Labels

status: accepted

Projects

None yet

Milestone

2.2.3

Development

Successfully merging this pull request may close these issues.

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

2 participants

@bryevdv @mattpap