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

Continuous vs discrete axes #1278

Open
axelboc opened this issue Nov 10, 2022 · 4 comments
Open

Continuous vs discrete axes #1278

axelboc opened this issue Nov 10, 2022 · 4 comments

Comments

@axelboc
Copy link
Contributor

axelboc commented Nov 10, 2022

Ticks

The axis configs we pass to VisCanvas may include a flag called isIndexAxis. This property has one goal: to prevent ticks from appearing between axis values - i.e. typically between image pixels on the heatmap:

isIndexAxis: true isIndexAxis: false
image image

We set isIndexAxis to true when we know the axes show pixel indices, that is:

  • on HeatmapVis, when custom abscissa/ordinate values are not provided
  • on LineVis, when custom abscissa values are not provided
  • on RgbVis, always for both axes (arguably this should follow the same behaviour as HeatmapVis now that we support custom axis values)

The assumption here is that when custom axis values are available, the axis is meant to be continuous.

When isIndexAxis is true, the Axis component calls getIntegerTicks instead of the axis' D3 scale's ticks() method, which cannot guarantee the generation of a specific number of ticks (i.e. scale.ticks(3) can actually generate more than 3 ticks...) This works great when dealing with indices ([0 ... cols]), but it doesn't work at all with custom axis values (e.g. getIntegerTicks([0.001, 0.004], 4) returns an empty array and the axis disappears...)

Of course, we can work around this problem easily in the viewer by setting isIndexAxis to false in RgbVis when custom absicssa/ordinate values are provided.

However, this does not prevent misuse of isIndexAxis by consumers of @h5web/lib.

Note also that getIntegerTicks does not prevent ticks from appearing outside of the bounds of the heatmap:

image

Tooltips

The other way in which the viewer currently supports discrete axes is in the tooltips of HeatmapVis and LineVis. In fact we always consider that the axes are discrete regardless of isIndexAxis: the tooltip's x and y values are always taken either from the axes' indices or from the custom values if provided:

Screenshot_2022-11-10_17-18-41

In other words, there is an inconsistency between the way the ticks are generated and the way the tooltip computes the coordinates of the value under the cursor. This is not great.

In Daiquiri, for the tomography visualizations, where isIndexAxis is left as false, the tooltips show continuous x and y values (in mm) as expected:

Screenshot_2022-11-10_17-18-41

Question

So the question this issue raises is: should we add proper support for discrete axes? Is this something that would be useful to some scientific fields, either in the viewer (via a toolbar toggle, or always enabled), or in the lib? By proper support, I mean:

  • not limited to integer ticks;
  • with a consistent behaviour across all relevant UI elements (i.e. ticks and tooltips, but we could also imagine in the future adjusting a zoom or mask selection to exact pixels automatically when discrete axes are enabled);
  • enabled with an API that is clear and prevents misuse.

Proper support is not trivial, since, for instance, axes would need more info than just a domain to generate discrete ticks: either the axis values, the number of values (i.e. number of pixels + 1), or the step between the values. Obviously, it would be better if axis values never had to be generated in the viewer, as this is a (minor) performance issue at the moment, so the number of values or the step feel like better solutions.

@axelboc axelboc changed the title Linear vs discrete axes Continuous vs discrete axes Nov 14, 2022
@loichuder
Copy link
Member

Early thoughts when reading this:

  • RgbVis should indeed follow the behaviour of HeatmapVis, now that it supports custom axes values
  • isIndexAxis could be renamed to restrictToIntegers or something similar to be more explicit
  • For the tooltip, it is a tricky question. In some cases, the value of the closest axis value is more interesting than the value of the actual coordinate under the mouse. And I am not sure this distinction can be summed up only by discrete vs. continuous ? Might need discussion.

@axelboc
Copy link
Contributor Author

axelboc commented Nov 14, 2022

If we decide to bring proper support for discrete ticks (i.e. not limited to integers), then we need more info than just a boolean: either the tick values (bad for perf), the tick count (purpose might be unclear for consumers) or the tick step (my preference). So if we go down this route, I'd recommend replacing isIndexAxis with a prop called step, or tickStep maybe (might as well be explicit).

For context, the renderTooltip function currently receives the continuous coordinates (x, y). It is then up to the consumer to decide whether to show the continuous coordinates directly or instead compute the discrete coordinates. Computing the discrete coordinates is currently done in the viewer by first generating an array of axis values and then converting the continuous coordinates to indices with a threshold scale. Not great... Knowing tickStep and the axis domain, we can avoid generating an array of axis values, we can remove the need for a threshold scale, and we can hide the continuous-to-discrete conversion logic inside TooltipMesh.

Perhaps we can make the tooltip behaviour consistent with the ticks by default and save the consumer a bunch of work by returning the discrete coordinates directly when tickStep is provided. Then, if we want to the consumer to keep the ability to have discrete tips but show continuous coordinates in the tooltip, then we could add a prop to TooltipMesh to opt out or opt in (e.g. forceContinuousCoords, or forceDiscreteCoords) of the default behaviour.

@loichuder
Copy link
Member

The concept of tickStep looks promising to me too. I am curious to see if it can remove the need for generation of axis values and/or threshold scales.

As for the tooltip, I wouldn't change much for now and iterate over the changes brought by tickStep in a second time.

@axelboc
Copy link
Contributor Author

axelboc commented Dec 6, 2022

We've received a couple of emails requesting the ability to use text strings and formatted timestamps (date/time) as axis labels. In both cases, I think this would require support for discrete axes. For timestamps, the email referred to the NetCDF Climate and Forecast convention, which talks about discrete axes

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

No branches or pull requests

2 participants