RFC: Proposal for new xy-chart package

Motivation
vx packages are (by design) low-level and modular which maximizes their flexibility, but also requires more work to create even simple charts. My library @data-ui/xy-chart is built on top of vx and was designed to make it easier to create common charts with less work. To solve this use-case within the vx ecosystem, and consolidate these efforts, @data-ui/xy-chart is being deprecated and we plan to port its functionality to a new vx package @vx/xy-chart.
Goals of this RFC

align on near- and mid-term features
align on an XYChart API
align on a general direction for implementation

Features
To have feature parity with @data-ui/xychart, the new package should support the following:

Computes and provides x- and y- scales across all data series
Handles mouse events for the XYChart across all data series

Positions tooltips and provides tooltip data
Supports programmatic tooltip control
Mouse events can be defined at chart or series level, can be disabled at series level

Supports the following *Series which are mostly wrappers around vx

Bar, Line, Point, Area, AreaDifference, StackedArea, StackedBar, GroupedBar, BoxPlot, ViolinPlot, Interval

Exports CrossHair component for use with Tooltip
Exports XAxis and YAxis components from vx
Supports horizontal and vertical reference lines
Supports x- and y-gridlines
Supports brush functionality (pre @vx/brush)
Supports styles with LinearGradient, Patterns, and a chart theme via props
Wraps individual points in FocusBlur handlers that are a11y accessible / tab-able

We would also like to add additional support for the following:
New near-term features

re-written in TypeScript
responsive by default
supports arbitrary datum shape + x / y data accessors (currently requires { x, y } datum shape)
first-class hooks support

New mid-term features

optionally render Tooltip in a Portal to fix z-index stacking context problem
easy creation of Legends
better support for overlays / annotations (points + regions)
integration with @vx primitives like brush, zoom, and drag
canvas support – vx is currently mostly svg based (this likely requires updates in other vx packages)
animation – @data-ui does not support animation. while this may not need to be fully baked in we should expose hooks to animate xy-chart

API
@techniq has done some great research on declarative react chart APIs here. Generally they are composable:
<Chart {...}>
  <Axis {...} />
  <Gridlines {...} />
  <Legend />
  <DataSeries {...} />
  <DataSeries {...} />
</Chart>

However there are some key differences:
data provided at the DataSeries level vs the chart container level

DataSeries level  – ✅ favored

() => <Chart><DataSeries data={...} /></Chart>

Pros 👍

it's more "natural" to directly link data to the series that will visually represent it
@vx/shape's (the basis for DataSeries) currently require data, so this is more consistent with separate package APIs
Series can use any custom logic they need for computing the x- and y- extent from their data, and the Chart container can simply collect these

additionally this allows more advanced functionality like horizontal orientation to be pushed to the Series-level without requiring it to be implemented by all Series and the Chart container doesn't need to have any knowledge of it

Cons 👎

Chart container needs a way to access the data across all series

Chart container level  – ❌ disfavored

() => <Chart data={...}><DataSeries /></Chart>

Pros 👍

ultimately the Chart needs access to all data in order to provide x- and y- scales; this makes that easy.

Cons 👎

Series may require custom logic to compute x- and y- extent from their data (e.g., a bar stack) which the Chart needs to be aware of in this model
still requires key accessors at the Series-level for Series data
forces all DataSeries to have the same data length (or be filled with empty values)

Mouse and Touch events
Mouse and touch events are handled in several ways

Standard mouse events  – ✅ favored
react-vis and @data-ui expose mouse and touch events at both the Chart and Series level; these use standard react events such as onClick, onTouchMove, etc.

Custom event syntax  – ❌ disfavored
Some libraries like Victory have their own custom event system with non-standard syntax and selection language.

react hooks
I've not been able to find any react vis libraries which expose hooks. Feels like an opportunity on top of a render / component API 😏
Implementation
We'd like to improve upon the following limitations of @data-ui v1 implementation:

Written in TypeScript
@data-ui was written in JavaScript, but vx is now a TypeScript project and typings will be similarly useful for @data-ui.

Use react context over cloning children
@data-ui was implemented before the new / more robust 16.3 context API. Therefore  chart styles and shared scales are passed via props + React.cloneElement. Combined with hooks, using context should open up a whole new set of API possibilities (see below).

What is kept in context
The function of the XYChart wrapper largely equates to managing shared state across the elements of a chart, which components can leverage as needed. . This includes

chart theme + styles
xScale that accounts for data range of all chart series and chart width + margin
yScale that accounts for data range of all chart series and chart height + margin
tooltipData + tooltipCoords, when applicable

In addition to chart width + height, the Chart container must have knowledge of all data (+ annotation) values to compute scales appropriately. Rather than having the Chart introspect props from child DataSeries or Annotations (which can get gnarly) we propose that DataSeries and Annotations register their data, xValues, and yValues in context.
This pushes the logic of data <> x/y extent mapping to DataSeries rather than the Chart, and allows the Chart to leverage these values to compute scales properly. It could look something like
// in e.g., <LineSeries {...props} />
const { key, data, xAccessor, yAccessor } = props;
const { registerData } = useContext(XYChart);

registerData({
  dataKey: key,
  xValues: data.map(d => xAccessor(d)),
  yValues: data.map(d => yAccessor(d)),
  // other interesting things to do at this level
  mouseEvents: false,
  legendItemRenderer,
});

Unknowns
I'm unsure if there are major performance implications of using hooks ⚡ 🐌
Proposed API
// all of these items have access to the same Chart `context` which includes
// theme, dataRegistry, xScale, yScale, colorScale, tooltipData, tooltipCoords
const { ChartProvider, XYChart, Legend, Tooltip } = useChart({ theme, scaleConfig, ... }));

() => (
  {/* context provider */}
  <ChartProvider>
   {/**
     * Chart renders `svg` container and computes `x-` and `y-scale`s using the
     * data registry context. It is either passed `width`/`height`, or it uses
     * `@vx/responsive` for auto-sizing
     */}
    <XYChart>
     {/**
       * DataSeries register their data on mount. When `x-` and `y-scale`s
       * are computed and available in context they render data.
       */}
      <LineSeries key={dataRegistryKey} data={...} />

     {/**
       * Axes use `x-` or `y-scale`s from context based on orientation.
       */}
      <Axis orientation="left" />

      {/* Custom axis component could use `scale`s from context */}
      <CustomAxis />
    </XYChart>

    {/**
      * Tooltip is `html`-based so should be rendered outside the chart `svg`
      * It has access to `tooltipData` and `tooltipCoords` from context.
      */}
    <Tooltip renderInPortal={boolean} />

    {/**
       * Legend is `html`-based so should be rendered outside the chart `svg`.
       * It has access to all series via `dataRegistry` from context,
       * or we could add a legend renderer registry.
       */}
    <Legend  />
  </ChartProvider>
)

The same functionality could be provided in a component API:
import { ChartProvider, XYChart, Legend, Tooltip } from '@vx/xy-chart`

() => (
  <ChartProvider theme={...} {...scaleConfig} >
    <XYChart {...} />
    <Legend {...} />
    <Tooltip {...} />
  </ChartProvider>
)

cc @hshoff @techniq @kristw

First, excellent writeup!  You concisely and completely covered very well.
Question, in the Proposed API example, the Fragment wrapping everything should be a ContextProvider (maybe call it ChartProvider)?.
I figure we'll want to start working on a PoC (maybe in codesandbox or something)?

Another question, what about Hierarchial layouts (Treemap, Partition (Sunburst, Icicle), Tree, Pack, Sankey, etc).  I assume instead of XYChart we would have HierarchyChart or similar?  Also for Radial variants, does it make sense to still call it XYChart.  For Semiotic they have XYFrame, OrdinalFrame, and NetworkFrame.

Another question, what about Hierarchical layouts (Treemap, Partition (Sunburst, Icicle), Tree, Pack, Sankey, etc). I assume instead of XYChart we would have HierarchyChart or similar? Also for Radial variants, does it make sense to still call it XYChart. For Semiotic they have XYFrame, OrdinalFrame, and NetworkFrame.

Good questions. I propose this RFC should limit the scope to XYChart since it is already a large enough topic, to prevent scope creeping.

@kristw Agreed, and if the plan is to handle it as a separate component (HierarchyChart/etc) we can do that in the future.

Question, in the Proposed API example, the Fragment wrapping everything should be a ContextProvider (maybe call it ChartProvider or maybe even just Chart)?.

Yes! My mistake, I've updated it to ChartProvider. I think that we'll likely need both ChartProvider and XYChart since things like <Legend /> and <Tooltip /> shouldn't be inside the svg. I guess they could be children of XYChart and be rendered in portals 🤔 but that seems complicated for legend placement.

I figure we'll want to start working on a PoC (maybe in codesandbox or something)?

Also agree, I think a POC sandbox should be easy to create from the vx codesandbox examples integration. I'll start a branch / PR and post back here 🎉

what about Hierarchical layouts ... I assume instead of XYChart we would have HierarchyChart? ... for Radial variants, does it make sense to still call it XYChart? for Semiotic they have XYFrame, OrdinalFrame, and NetworkFrame

Love the idea of a HierarchyChart in the future after we get this one working. Would need to think about Radial variants inside XYChart vs a separate RadialChart, or combining them into a single Chart. I think that Semiotic made the distinction between XY, Ordinal, and Network as fundamental data primitives, which make animating between different Series for a given frame easier (e.g., Bar series + Pie series in an Ordinal frame are fundamentally the same pieces of data with different visual representations). Maybe we should consider XY vs Ordinal ourselves 🤔  cc @emeeks

Features

Handles mouse events for the XYChart across all data series
Positions tooltips and provides tooltip data
Supports programmatic tooltip control
Mouse events can be defined at chart or series level, can be disabled at series level
Supports the following *Series which are mostly wrappers around vx: Bar, Line, Point, Area, AreaDifference, StackedArea, StackedBar, GroupedBar, BoxPlot, ViolinPlot, Interval

Exports CrossHair component for use with Tooltip
Exports XAxis and YAxis components from vx
Supports horizontal and vertical reference lines
Supports x- and y-gridlines
Supports brush functionality (pre @vx/brush)
Supports styles with LinearGradient, Patterns, and a chart theme via props
Wraps individual points in FocusBlur handlers that are a11y accessible / tab-able

re-written in TypeScript 🔥
responsive by default 🔥
supports arbitrary datum shape + x / y data accessors (currently requires { x, y } datum shape) 🔥
first-class hooks support 🔥

All the good stuffs!
Comment #1

Computes and provides x- and y- scales across all data series

I would like to propose one change from the original @data-ui library. The original library takes scale config  object and pass it to @vx/scale to create a scale. I wish the XYChart would take D3 scales instead. This way the logic for constructing a scale would be left up to the developer whether to use @vx/scale or not.
Full disclosure: I have been developing the encodable library with a createScaleFromConfig() function, which take the scale config object with similar schema to vega and vega-lite and return a D3 scale. This has more features that the current @vx/scale factories (such as ensuring scales starting at zero, nicing with specified time units, creating piecewise scales that matches number of domain and range items for sequential color scales.) Currently, I ended up having to reverse engineer the output D3 scales back to @vx/scale config objects (which supports a subset of the configuration) to be able to use XYChart.
Comment #2

data provided at the DataSeries level vs the chart container level

Here are a few additional alternatives. I supposed the data mentioned in the RFC is always thought of as an array, but perhaps it does not have to be.
Option 3a. This means the data have to be passed twice, with the provided coding convention below.
const dataset = {
  cost: [{ x: 1, y: 2, datum: ... }, { x: 3, y: 4, datum: ... }],
  revenue: [{ x: 1, y: 2, datum: ... }, { x: 3, y: 4, datum: ... }],
};

<XYChart dataset={dataset} >
  <LineSeries key="cost" data={dataset[cost]} />
  <LineSeries key="revenue" data={dataset[revenue]} />
</XYChart>

Option 3b. The data is passed once to the chart, with series referring to its data via key or dataKey (if provided).
<XYChart dataset={{
  cost: [{ x: 1, y: 2, datum: ... }, { x: 3, y: 4, datum: ... }],
  revenue: [{ x: 1, y: 2, datum: ... }, { x: 3, y: 4, datum: ... }],
}} >
  // implicitly use "cost" as dataKey
  <LineSeries key="cost" />
  // implicitly use "revenue" as dataKey
  <LineSeries key="revenue" />
  // Use the same data with the line series above.
  <BarSeries key="revenue-bar" dataKey="revenue" />
</XYChart>

Pros

Chart have access to all data in order to provide x- and y- scales.
Can directly link data to the series that will visually represent it
Does not force all DataSeries to have the same data length.
(3a) More transparency for the developer about the data passed to the chart vs series.

Cons

The data has to be processed to compute x and y instead of being able to define xAccessor and yAccessor at the Series level. On the bright side, this will push caching/memoization one level above and results are sharable between series that use the same data.
(3a) Looks a bit duplicative. Perhaps can be refined to be more concise somehow.
(3b) Series have to access dataset from XYChart parent, but perhaps this is not really a con since it needs access to scales anyway.

Comment-3
Now the XYChart magically computes and sets domain for the entire chart based on all given data. There might be times when developers wish to override the domain computation, such as to add min/max bounds, zeroing, manually specify domain. Could we add one more optional hook to the XYChart props which sets the default value to the current behavior.
type Props = {
  ...,
  setDomainFromDataset: ({ dataset, xScale, yScale }) => void;
}

setDomainFromDataset({ dataset, xScale, yScale }) {
  // computes and sets xScale.domain(...)
  // computes and sets yScale.domain(...)
}

I would like to propose one change from the original @data-ui/xy-chart library. The original library takes scale config object and pass it to factory functions from @vx/scale to create a scale. I wish the XYChart would take D3 scales instead. This way the logic for constructing a scale would be left up to the developer whether to use @vx/scale or not.

Hmm, to me this adds a lot of eng effort + a learning curve which detracts a bit from the goal of "easy to use out of the box". If you pass in a scale directly, the chart has to either mutate or copy it, which I don't think is as simple as passing in scale config and having the chart create the right scale.
I think the scale config in @data-ui is a little black-box because it wasn't in TypeScript. Maybe we can add some of the config customizations you are thinking about to the config? (would this be a lot of work to port over from encodable?) I think it's reasonable for the chart to use @vx/scale if it's a vx package (which really are just d3 scales).

Now the XYChart magically computes and sets domain for the entire chart based on all given data. There might be times when developers wish to override the domain computation, such as to add min/max bounds, zeroing, manually specify domain. Could we add one more optional hook to the XYChart props which sets the default value to the current behavior.

XYChart currently allows you to pass any d3-scale/@vx/scale config you want, including a custom range or domain for this purpose: xScale={{ type: 'linear', domain: [35, 100] }}. I think exposing the config directly is more straight forward than adding a prop for each config hook (like computeDomain, computeRange).

Hmm, to me this adds a lot of eng effort + a learning curve which detracts a bit from the goal of "easy to use out of the box". If you pass in a scale directly, the chart has to either mutate or copy it, which I don't think is as simple as passing in scale config and having the chart create the right scale.

I am not sure which one is easier to learn. If vx = react + D3 I supposed the developers are already familiar with D3.
import { scaleLinear } from 'd3-scale';
<XYChart xScale={scaleLinear().domain([0, 100]).nice(true)} />

vs. learning another construct which is specific to vx in the current implementation.
<XYChart xScale={{ type: 'linear', domain: [0, 100], nice: true }} />

I am not saying we strongly remove this, but perhaps it can be opt-in.
import { scaleLinear } from 'vx-scale';
<XYChart xScale={scaleLinear({ domain: [0, 100], nice: true })} />

XYChart currently allows you to pass any d3-scale/@vx/scale config you want, including a custom range or domain for this purpose: xScale={{ type: 'linear', domain: [35, 100] }}. I think exposing the config directly is more straight forward than adding a prop for each config hook (like computeDomain, computeRange).

This is related to the above proposal if the XYChart accept D3Scale instance (not just a config) then the fixed domain information is not available. e.g. if scale.domain() returns [0, 1], it is not clear if this is the default domain or the fixed domain specified by user.
Example scenarios:

Example 1: how to ensure that y-axis starts from 0 and max is max value from dataset? Don't see how to configure in config. https://github.com/hshoff/vx/blob/master/packages/vx-scale/src/scales/linear.ts
Example 2: if I want the domain to be [0, 95th percentile]
Example 3: if I want to exclude some series from domain computation.
Example 4: want domain to always be [0, 1] (such as percentage)

This is probably an advanced feature tho so usually new developers won't touch. Arguably for example 1-3, the domain could be computed beforehand and passed into XYChart as part of the xScale and yScale. Not sure how to deal with the edge case in example 4.

(would this be a lot of work to port over from encodable?)

There are possibilities. The part that make it a bit tangle is ability to lookup color scheme by names in encodable. If removed that part, can probably port over.

Regarding custom scales with percentages, we do this in our apps as well, either as 0-100%...

or with 100% as a baseline...

we also have a yScaleZero prop to always show 0 and not "zoom to fit" the data.  We do not use XYChart in data-ui but have our own Charts using vx primatives, but would love to move to this composable components as well.
There are also some primatives we have such as OverlayBandBar that is used to 'highlight' on hover that will be much easier and composible when the scales are available via context (and not be required to pass in directly

interface OverlayBarProps {
  x: number;
  xScale: ScaleBand<any>;
  yScale: ScaleLinear<any, any>;
}

function OverlayBandBar(props: OverlayBarProps) {
  const { x, xScale, yScale } = props;

  return (
    <Bar
      x={x - (xScale.padding() * xScale.step()) / 2}
      y={0}
      width={xScale.step()}
      height={yScale.range()[0]}
      fill="rgba(0,0,0,.1)"
    />
  );
}

We also have OverlayTimeBandBar, OverlayTimeBar, and OverlayTimeLine to do similar based on scales.

^awesome examples @techniq 😍  I agree we should support all of these, it should be straight forward with context. Wanted to update that I have a working POC (sandbox link):

I'll open a PR soon for ease of commenting. I'm trying to work out more of the details for events. Some initial notes:
Mapping events <> Datum
data-ui supports different event triggers: container, voronoi, and series. These are useful under different scenarios: voronoi works well for points + lines but not when there are series where single points take up a lot of space like Bar or BoxPlot. Here series works but container is better for e.g., small Bars with small mouse targets. The implementation of container looks for the closest x,y point, but this really still doesn't work well for bars or when supporting e.g., vertical + horizontal bars since these take.
General user feedback (from non-vis engineers) was that the 3 options were confusing and they didn't understand when to use what 😢 So I'm thinking about how we could improve the dev experience by doing this automatically. I think generally we could

collect all data { x, y }s and use a d3-quadtree for fast lookup at the container level.
for cases like Bar or HorizontalBar, we could come up with a mechanism for the Series to override the default logic 🤔 still thinking.

Event context
In thinking more about perf, since any child who uses a given context updates when a context value changes, I think it might be preferable to have a separate context for events, which <Tooltip /> or other components could tap into separately from ChartContext. This would have the benefit of not updating every ChartContext consumer from updating every mouse move.

Regarding tooltips and bar charts, one thing I do is render a BarTarget under actual Bar to allow the target to be the full band (instead of having dead zones between the bars, and to go the the top of the chart height.
For example on the our BarChart, we render a <VerticalBarShape> which includes both the visible shape and the hit target (see the band example above and you get the idea of the hit target).
const VerticalBarShape: React.FunctionComponent<VerticalBarShapeProps> = ({
  bar,
  index,
  xScale,
  yScale,
  color,
  barProps,
  barTargetProps,
  showTooltip,
  hideTooltip
}) => {
  const [yMax] = yScale.range();

  return (
    <React.Fragment key={`bar-${bar.x}`}>
      <Bar
        x={bar.x}
        y={bar.y}
        width={bar.width}
        height={bar.height}
        fill={color || bar.color || blue[500]}
        {...(barProps && barProps({ bar, index }))}
      />
      <BarTarget
        bar={bar}
        x={bar.x - (xScale.padding() * xScale.step()) / 2}
        y={0}
        width={xScale.step()}
        height={yMax}
        showTooltip={showTooltip}
        hideTooltip={hideTooltip}
        // fill="rgba(255,0,0,.1)"
        {...(barTargetProps && barTargetProps({ bar, index }))}
      />
    </React.Fragment>
  );
};

we also have a HorizontalBarShape
const HorizontalBarShape: React.FunctionComponent<HorizontalBarShapeProps> = ({
  bar,
  index,
  xScale,
  yScale,
  color,
  barProps,
  barTargetProps,
  showTooltip,
  hideTooltip
}) => {
  const [xMin, xMax] = xScale.range();

  return (
    <React.Fragment key={`bar-${bar.x}`}>
      <Bar
        x={bar.x}
        y={bar.y}
        width={bar.width}
        height={bar.height}
        fill={color || bar.color || blue[500]}
        {...(barProps && barProps({ bar, index }))}
      />
      <BarTarget
        bar={bar}
        x={0}
        y={bar.y - (yScale.padding() * yScale.step()) / 2}
        width={xMax}
        height={yScale.step()}
        showTooltip={showTooltip}
        hideTooltip={hideTooltip}
        {...(barTargetProps && barTargetProps({ bar, index }))}
      />
    </React.Fragment>
  );
};

lastly, sometimes we override the "full band" targets where it makes sense, for example on a TimelineChart (Gnatt chart).  It this case we just wante the target to be the size of the bar (plus the circles on the sides which gives it a "pill" shape.
<Fragment key={`${y(d)}-${i}`}>
  <HorizontalBarShape
    bar={bar}
    index={i}
    xScale={xScale}
    yScale={yScale}
    color={color ? color(d, i) : undefined}
    barProps={barProps}
    barTargetProps={({ bar, index }) => ({
      x: bar.x - pointRadius,
      width: bar.width + pointRadius * 2,
      ...(barTargetProps && barTargetProps({ bar, index }))
    })}
    showTooltip={showTooltip}
    hideTooltip={hideTooltip}
  />

  {endpoints && (
    <>
      <circle
        key={'start-' + start.toString()}
        fill={color ? color(d, i) : undefined}
        cx={xScale(start)}
        cy={yScale(y(d)) + pointRadius}
        r={pointRadius}
        style={{ pointerEvents: 'none' }}
        {...(endpointProps &&
          endpointProps({ point: start, index: 0, bar }))}
      />

      <circle
        key={'end-' + end.toString()}
        fill={color ? color(d, i) : undefined}
        cx={xScale(end)}
        cy={yScale(y(d)) + pointRadius}
        r={pointRadius}
        style={{ pointerEvents: 'none' }}
        {...(endpointProps &&
          endpointProps({ point: end, index: 1, bar }))}
      />
    </>
  )}
</Fragment>

I'm not sure how helpful these snippets are but sadly I don't have an example in codesandbox right now.

Regarding the separate event context, that seems like a good idea for performance reason (unless we can do a lot of memoization / etc, but taking away as many foot guns as possible is always good.

Wanted to get resolution on a couple of high-level directions while I continue on the POC:
Resolution on data at the Chart container level versus the DataSeries level
@kristw proposed a couple more options for pushing data to the Chart level. I think this is appealing in the sense that you might avoid the need  for Series to pass data back up to the Chart container, but I still think that we should opt for data at the DataSeries level.
I think the major limitation at the Chart level, even if you use an object instead of arrays, would be that you to either

have the same x,y accessors for all data, or
set them on individual DataSeries

The first point is quite limiting, and the second feels a little disjointed / inconsistent. Additionally, if you are going to add accessors to DataSeries you'd need a mechanism to register them with the Chart container ... and at that point you still need the data registry and should just co-locate the data + accessors.
Resolution on scales vs scale config
@kristw was proposing passing scales directly over configuration and gave this set of examples

Example scenarios:
Example 1: want to ensure that y-axis starts from 0 and max is max value from dataset. Don't see how to configure in config. https://github.com/hshoff/vx/blob/master/packages/vx-scale/src/scales/linear.ts
Example 2: want the domain to be [0, 95th percentile]
Example 3: want to exclude some series from domain computation.
Example 4: want the domain to always be [0, 1] (such as percentage)
Arguably for example 1-3, the domain could be computed beforehand and passed into XYChart as part of the xScale and yScale. Not sure how to deal with the edge case in example 4.

I would still advocate for scale config (with types for easier use/dev experience) for 2 reasons.

Config like the following would allow you to implement any of the 4 examples by specifying your own domain. I think this is more straight forward than a user passing in a scale where it's unclear what should be mutated by the Chart vs what has already been set by the user. Perhaps we could also allow you to pass in a scale directly but that seems trickier to type check.

// this is always true
type ScaleOutput = number[];

// in reality we probably need to make this a union of different configs per type
interface ScaleConfig<ScaleInput> {
  type: 'linear' | 'band' | 'time' | ... ;
  domain?: ScaleInput[];
  range?: ScaleOutput[];
  rangeRound?: ScaleOutput[];
  nice?: boolean;
  clamp?: boolean;
}

The second reason mostly comes down to this question

I am not sure which one is easier to learn. If vx = react + D3, I supposed the developers are already familiar with D3.

I think most vx users are actually not familiar with d3. Many of the issues that are opened come down to not understanding d3 behavior, so I think having them use config (which feels more like react) versus having them learn d3 scales I think would be preferable / simpler. If the config exposes all of the d3 configuration options I think a d3 developer should be able to easily pick it up.

wdyt?

RE: Resolution on data at the Chart container level versus the DataSeries level

@kristw proposed a couple more options for pushing data to the Chart level. I think this is appealing in the sense that you might avoid the need for Series to pass data back up to the Chart container, but I still think that we should opt for data at the DataSeries level.

I feel less strongly about this one and can go either way. My main concern was the challenges for library maintainer to handle the performance optimization, but can see why it can be more developer-friendly. (And since the maintainer is willing to handle the complexity, I will not discourage. 😁 )
Notes:

The container/registry can do some state management to avoid duplicate datasets from series to optimize computing time.
Have to more selector/memoize to avoid updating every series/rendering when the context changes.
There will be edge case for computing extents when the series have multiple x/y-values anyway. Perhaps the per-series approach is better for this. Implementation-wise, each series have more options to feed into the registry.

RE: Resolution on scales vs scale config

Object config feels more like react to me and is significantly more readable than the d3 api, which is very foreign to newcomers used to react. Additionally this saves the developer the effort of having to import another thing from the package and use it correctly versus...

Agree with making it prioritized for react-oriented developers, but also would like to have support for advanced use cases and/or open the door for d3-oriented developers. We should be able to do the best of both worlds?
I personally ran into the situation that I already have a D3 scale and had to choose between (a) convert my D3 scale to the config, only for it to be reconstructed as a D3 scale again with subset of the behavior I had in my original scale (b) not using XYChart at all. Wish these were not the choices that were given.

@kristw was proposing passing scales directly over configuration

Let me clarify a bit. My original proposal is NOT getting rid of the config, but extends the xScale and yScale fields to also accept D3 scales somehow. I was tossing around the idea which can be out of context. Let me reframe it here.
Option A: The current behavior
type XScale = {
  type: 'linear' | 'band' | 'time' | ... ;
  domain?: ScaleInput[];
  range?: ScaleOutput[];
  rangeRound?: ScaleOutput[];
  nice?: boolean;
  clamp?: boolean;
}

<XYChart xScale={{ type: 'linear', ... }} />

Cons: I can't use my own D3 scale and workarounds are not satisfying.
Option B: Also allow D3 scales as input
import {
  ScaleLinear,
  ScaleLogarithmic,
  ScalePower,
  ScaleTime,
  ScalePoint,
  ScaleBand,
} from 'd3-scale';

type SupportedD3Scale =
  | ScaleLinear
  | ScaleLogarithmic
  | ScalePower
  | ScaleTime
  | ScalePoint
  | ScaleBand

type XScale = {
  type: 'linear' | 'band' | 'time' | ... ;
  domain?: ScaleInput[];
  range?: ScaleOutput[];
  rangeRound?: ScaleOutput[];
  nice?: boolean;
  clamp?: boolean;
} | SupportedD3Scale;

// You can pass config
<XYChart xScale={{ type: 'linear', ... }} />
// or You can pass scale.
<XYChart xScale={scaleLinear()} />

Pros: No breaking change.
Cons: Unclear if the input scale should update domain from the data. Needs a way to specify that.
Option C: Only allow D3 scales as input
This is a slight variation of the Option B.
// when using config
import { createScale } from '@vx/scale')
<XYChart xScale={createScale({ type: 'linear' })} />

// when using scale
import { scaleLinear } from 'd3-scale')
<XYChart xScale={scaleLinear()} />

Pros: uniform input for the XYChart component.
Cons:

Breaking behavior.
Extra call to import @vx/scale.
Unclear if the input scale should update domain from the data. Needs a way to specify that.

Option D: Let this be another type
type XScale = {
  type: 'linear' | 'band' | 'time' | ... ;
  domain?: ScaleInput[];
  range?: ScaleOutput[];
  rangeRound?: ScaleOutput[];
  nice?: boolean;
  clamp?: boolean;
} | {
  type: 'custom',
  scale: SupportedD3Scale;
  updateDomain: boolean | (scale, domainFromData) => SupportedD3Scale,
  updateRange: boolean | (scale, dimension) => SupportedD3Scale,
}

<XYChart xScale={{
  type: 'custom'
  scale: myOwnScale,
  updateDomain: false,
  updateRange: true
}} />

I think this looks safe. The custom type sounds a bit weird, but I am OK with this more than being boxed by the config.
RE: Other smaller points
Checking if an input is a scale config or D3 scale.

Perhaps we could also allow you to pass in a scale directly but that seems trickier to type check.

Can check if .domain field exists and if it is a function.
@vx/scale config can do everything a D3 scale does

If the config exposes all of the d3 configuration options I think a d3 developer should be able to easily pick it up.

This requires @vx/scale to maintain 100% feature parity with d3-scale, which is not the case at the moment. e.g. the unknown function. I am not saying the unknown function is very important, but the point is there will be a time when they are out of sync and the developer will be like what am I supposed to do if I want the new feature.
If you ensure 100% parity and the developer should send PRs/wait for next release, that signals a forever contract for the maintenance of @vx/scale for every new/change features in d3 scales. And if they are exactly the same, then why can't a developer pass a D3 scale if he/she already have it by other means.
Making @vx/scale better
// this is always true
type ScaleOutput = number[];

// in reality we probably need to make this a union of different configs per type
interface ScaleConfig<ScaleInput> {
  type: 'linear' | 'band' | 'time' | ... ;
  domain?: ScaleInput[];
  range?: ScaleOutput[];
  rangeRound?: ScaleOutput[];
  nice?: boolean;
  clamp?: boolean;
}

I have written all of these already in
https://github.com/apache-superset/encodable/blob/master/packages/encodable/src/types/Scale.ts
I can take over @vx/scale and do major updates too if you are OK with it.

Something else to consider about scales is animating.  I've been experimenting with this again (mostly with hierarchy charts but also zooming scales for time series, etc) you can see here:

https://codesandbox.io/s/github/techniq/vx-hierarchy-examples/.

Currently I interpolate/tween the values using d3-interpolate and call scale.domain(...).range(...) in the Spring onFrame function.  There might be a better approach for some of the caresian layours, but for Sunburst, I think this currently is the best approach.
I haven't checked in some of my recent changes (working on a zoomable Treemap and adding other hierarchy layouts, still just experimenting though).
I'm hoping to uncover a good pattern / custom animated scale hook for this at some point.

I just had a chance to look at the vx-brush example as I thought it would be similar to animation, although it looks like you build a new scale instance (utilizing useMemo() and deps) instead of mutating the original instance.  I guess this would port to a scale config since you would just be sending a new config to vx-chart whenever the brush changes.  Not sure this could work for animations though since react-spring makes these changes outside the react render updates (for performance reasons).

@kristw I think it'd be awesome to improve the typings in @vx/scale, those are a lot of work so would love your help with that and improving the package more generally 😄
@techniq thanks for highlighting more of the complexities with animations and scale updates, I agree we should flesh that out more. your approach is really interesting, that seems like a pretty good API to me overall (we should try to update the brush example to use that approach, we're definitely just brute-forcing it right now 😬 ).
A problem I've not known how to solve with updating scale domain/range is inducing a child component update. In an XYChart setting where the Provider has the scales, my initial thought was that you'd need to create new scale instances in order to properly trigger children to re-render with the new values. It looks like in your hook only the interpolate value would be re-created each cycle? May need to noodle on this more.

Separately, it'd be sweet to add patterns/things like useAnimatedScale to @vx/scale over time to make it more distinct and useful in react land than the existing d3 wrappers. @hshoff and I also were contemplating exporting memoized scales, with dependencies, since you often need them in render methods (esp function components) and have to always do the memoization yourself (or face the perf consequences of not doing that).

@williaster With interpolate, it's leveraging react-spring to skip the re-renders.  This is also why you have to use animated.* for the DOM (animated.rect, animated.g, etc).  This is mentioned breifly on their docs.  It's also one of the big benefits over react-move which did these changes with state-only.
I know the react-spring organization also has their own state management call zustand and one of their features is Can inform components transiently (without causing render) and they also do not use react context.  I've not used it myself or know much more about it though (and it's more for global state ala-redux if I'm not mistaken).  Might be worth looking at their implementation and/or using it ourselves (not sure if we want the depenedency though).
I also just read up a little more as I was curious how zustand works with Concurrent React.  It sounds like they are still debating but there was some promise with a new hook added to React (useMutableSource) although it sounds like they are still having issues with it.  We might be able to use this hook directly though.

Regarding memoizing, I've been waiting to mention this, but it seems like we should memoize all our hierarchy layouts such as Treemap, Partition, etc.  I think this is were the slight delay in my examples is coming from after clicking on a child (it's recalculating the layout on each render).  As that point we could also expose them as hooks useTreemap()/etc.

There is also react-tracked that sounds promosing.  It also looks to have been ported to useMutableSource.

Have several pieces working with a different animation approach, didn't quite get to the PR today

@williaster

Okay finally! (sorry there were some problems with other vx packages that I fixed locally, that then wouldn't work in the sandbox)
Here's the PR #745 🎉 Have a lot of things working (love the dark mode + animation!), really curious about your feedback on the react-spring approach. I tried this approach and your useAnimatedScale variant but couldn't actually get that to interpolate/tween values correctly. Can share that exploration if you want.

Going to close this issue as implementation is underway. You can follow progress in the project tracker.


[fix] support cuGetProcAddress and cuGetProcAddress_v2

hook cuGetProcAddress_v2
cuGetProcAddress and _v2 serve as lib entry point

@grgalex PTL

@pokerfaceSad Thanks for making the changes!
I see that you didn't apply changes (3) and (6) from my suggestions (https://github.com/grgalex/nvshare/pull/13#issuecomment-1845775314), but I can do this for you :)
Also, next time, you can use git reset to undo the commits on your branch, then redo them and finally git push --force to change the commits in your PR, without having to close it and open a new one.

@pokerfaceSad Thanks for making the changes!
I see that you didn't apply changes (3) and (6) from my suggestions (https://github.com/grgalex/nvshare/pull/13#issuecomment-1845775314), here are the commit messages I suggest:
1.
hook: Add conditional library initialization in cuGetProcAddress()

initialize_libnvshare() is our bootstrapping function, which, among others,
obtains the original CUDA function addresses needed for our hook logic.

We currently only call initialize_libnvshare() when the user application
makes its first call to cuInit(), which we assume is **always** the first
CUDA Driver API function that it makes.

However, CUDA >=11.3 applications that use the Runtime API call
cuGetProcAddress() before cuInit(), which means that in these
cases cuGetProcAddress() is the first function called.

So far we were lucky, as the first call that such apps make is
cuGetProcAddress("cuInit"), which falls into our custom handling cases
and does not invoke the original cuGetProcAddress() to which we don't
have a pointer, as we haven't bootstrapped CUDA yet.

To address the aforementioned case and avoid potential NULL pointer
dereferences, add a one-time call to initialize_libnvshare()
in cuGetProcAddress().

2.
hook: Add hook for cuGetProcAddress_v2() to support CUDA >=12

CUDA 12.0 introduced a new function, cuGetProcAddress_v2(), which Runtime
API applications call instead of cuGetProcAddress() in order to obtain
Driver API symbols.

To maintain compatibility with CUDA >=12.0 applications,
add a hook for cuGetProcAddress_v2().

Closes #11

You don't have to create a new PR for this.
You can do the following to amend the current PR.

Use git rebase -i HEAD~2
Specify the reword (r) action on the two commits
Change the commit messages (Don't forget to keep the Signed-off-by: line at the end, after the Closes #11 line, with one empty line inbetween).
Force-push git push --force your changes to your branch.

The PR here will be automatically updated!

@grgalex Done.
Thanks for your help and the detailed explanation. :)

@pokerfaceSad I've made some changes and pushed to the pokerface-rev branch (of this repo).
Do you have access to a GPU machine, so you can test the changes we made?
Can you build and test on Pytorch 2.1.0 with the snippet provided at https://github.com/grgalex/nvshare/issues/11#issue-1985782574?
If we really want to be sure, we should also add a gdb breakpoint for cuGetProcaddress_v2 and prove that the hooking is successful.
After you are done testing, I will build and update the Docker image tags in a separate commit.

@pokerfaceSad You should add your tencent mail (which you used to author the commits) to your GH profile so the commits get attributed to your account properly.

@grgalex
Remove L604, or it will lead to segmentation fault.
After that it will work well on torch 2.0 / CUDA 12.1 / Driver 450.80.02.
root@e8c9779fe01b:/# NVSHARE_DEBUG=1 LD_PRELOAD=/libnvshare.so ./cumem
[NVSHARE][DEBUG]: Found NVML
[NVSHARE][DEBUG]: NVSHARE_POD_NAME = none
[NVSHARE][DEBUG]: NVSHARE_POD_NAMESPACE = none
[NVSHARE][DEBUG]: Sent REGISTER
[NVSHARE][DEBUG]: Received SCHED_ON
[NVSHARE][INFO]: Successfully initialized nvshare GPU
[NVSHARE][INFO]: Client ID = 1552bbb313cda0c8
[NVSHARE][DEBUG]: real_cuMemGetInfo returned free=32200.75 MiB, total=32510.50 MiB
[NVSHARE][DEBUG]: nvshare's cuMemGetInfo returning free=30974.50 MiB, total=32510.50 MiB
[NVSHARE][DEBUG]: cuMemAlloc requested 200000 bytes
[NVSHARE][DEBUG]: cuMemAllocManaged allocated 200000 bytes at 0x7f599e000000
[NVSHARE][DEBUG]: Total allocated memory on GPU is 0.19 MiB
[NVSHARE][DEBUG]: cuMemAlloc requested 200000 bytes
[NVSHARE][DEBUG]: cuMemAllocManaged allocated 200000 bytes at 0x7f599e030e00
[NVSHARE][DEBUG]: Total allocated memory on GPU is 0.38 MiB
[NVSHARE][DEBUG]: cuMemAlloc requested 200000 bytes
[NVSHARE][DEBUG]: cuMemAllocManaged allocated 200000 bytes at 0x7f599e061c00
[NVSHARE][DEBUG]: Total allocated memory on GPU is 0.57 MiB
[NVSHARE][DEBUG]: Received LOCK_OK
[NVSHARE][DEBUG]: Pending Kernel Window is 2.
Test PASSED
[NVSHARE][DEBUG]: Total allocated memory on GPU is 0.38 MiB
[NVSHARE][DEBUG]: Total allocated memory on GPU is 0.19 MiB
[NVSHARE][DEBUG]: Total allocated memory on GPU is 0.00 MiB
Done
root@e8c9779fe01b:/# vi test.py
root@e8c9779fe01b:/# LD_PRELOAD=/libnvshare.so NVSHARE_DEBUG=1 python test.py
[NVSHARE][DEBUG]: Found NVML
[NVSHARE][DEBUG]: NVSHARE_POD_NAME = none
[NVSHARE][DEBUG]: NVSHARE_POD_NAMESPACE = none
[NVSHARE][DEBUG]: Sent REGISTER
[NVSHARE][DEBUG]: Received SCHED_ON
[NVSHARE][INFO]: Successfully initialized nvshare GPU
[NVSHARE][INFO]: Client ID = 04d8423318bd930e
[NVSHARE][DEBUG]: real_cuMemGetInfo returned free=32200.75 MiB, total=32510.50 MiB
[NVSHARE][DEBUG]: nvshare's cuMemGetInfo returning free=30974.50 MiB, total=32510.50 MiB
[NVSHARE][DEBUG]: cuMemAlloc requested 2097152 bytes
[NVSHARE][DEBUG]: cuMemAllocManaged allocated 2097152 bytes at 0x7f056c000000
[NVSHARE][DEBUG]: Total allocated memory on GPU is 2.00 MiB
[NVSHARE][DEBUG]: Received LOCK_OK
[NVSHARE][DEBUG]: cuMemAlloc requested 1024 bytes
[NVSHARE][DEBUG]: cuMemAllocManaged allocated 1024 bytes at 0x7f056c200000
[NVSHARE][DEBUG]: Total allocated memory on GPU is 2.00 MiB
[NVSHARE][DEBUG]: cuMemAlloc requested 131072 bytes
[NVSHARE][DEBUG]: cuMemAllocManaged allocated 131072 bytes at 0x7f056c200400
[NVSHARE][DEBUG]: Total allocated memory on GPU is 2.13 MiB
[NVSHARE][DEBUG]: cuMemAlloc requested 8388608 bytes
[NVSHARE][DEBUG]: cuMemAllocManaged allocated 8388608 bytes at 0x7f056c400000
[NVSHARE][DEBUG]: Total allocated memory on GPU is 10.13 MiB
[NVSHARE][DEBUG]: cuMemAlloc requested 20971520 bytes
[NVSHARE][DEBUG]: cuMemAllocManaged allocated 20971520 bytes at 0x7f056cc00000
[NVSHARE][DEBUG]: Total allocated memory on GPU is 30.13 MiB
[NVSHARE][DEBUG]: Pending Kernel Window is 2.
[NVSHARE][DEBUG]: Pending Kernel Window is 4.
[NVSHARE][DEBUG]: cuMemAlloc requested 1024 bytes
[NVSHARE][DEBUG]: cuMemAllocManaged allocated 1024 bytes at 0x7f056c220400
[NVSHARE][DEBUG]: Total allocated memory on GPU is 30.13 MiB
[NVSHARE][DEBUG]: cuMemAlloc requested 131072 bytes
[NVSHARE][DEBUG]: cuMemAllocManaged allocated 131072 bytes at 0x7f056c220800
[NVSHARE][DEBUG]: Total allocated memory on GPU is 30.25 MiB
[NVSHARE][DEBUG]: cuMemAlloc requested 8388608 bytes
[NVSHARE][DEBUG]: cuMemAllocManaged allocated 8388608 bytes at 0x7f0532000000
[NVSHARE][DEBUG]: Total allocated memory on GPU is 38.25 MiB
[NVSHARE][DEBUG]: Pending Kernel Window is 8.
Epoch 1, Loss: 1.614
[NVSHARE][DEBUG]: Pending Kernel Window is 16.
Epoch 2, Loss: 1.590
Epoch 3, Loss: 1.566
[NVSHARE][DEBUG]: Pending Kernel Window is 32.
Epoch 4, Loss: 1.544
Epoch 5, Loss: 1.522
Epoch 6, Loss: 1.500
[NVSHARE][DEBUG]: Pending Kernel Window is 64.
Epoch 7, Loss: 1.479
Epoch 8, Loss: 1.458
Epoch 9, Loss: 1.438
Epoch 10, Loss: 1.419
Epoch 11, Loss: 1.400
Epoch 12, Loss: 1.381
[NVSHARE][DEBUG]: Pending Kernel Window is 128.
Epoch 13, Loss: 1.363
Epoch 14, Loss: 1.345
Epoch 15, Loss: 1.328
Epoch 16, Loss: 1.311
Epoch 17, Loss: 1.295
Epoch 18, Loss: 1.279
Epoch 19, Loss: 1.263
Epoch 20, Loss: 1.248
Epoch 21, Loss: 1.233
Epoch 22, Loss: 1.219
Epoch 23, Loss: 1.204
Epoch 24, Loss: 1.191
Epoch 25, Loss: 1.177
[NVSHARE][DEBUG]: Pending Kernel Window is 256.

 I understand that we segfault because the application (in this case I believe it's an internal call from the CUDA Runtime library) provides a NULL value for symbolStatus.
However, it could be the case that an application actually uses symbolStatus and checks its return value.
By the way, did you use Pytorch 2.0 or 2.1.0 for the testing you did above?
It's important to use a Pytorch version that ships with CUDA Runtime >=12.0, which only holds for Pytorch versions >=2.1.0.....
Additionally, could you please examine using gdb the value that the aforementioned application that you ran passes to symbolStatus?
To me, the cleanest approach for us would be to check if symbolStatus == NULL before attempti to set it.

@pokerfaceSad Any chance you had some time to look at this?

@grgalex Sorry, I've been quite busy recently.🥱
You are right. The CUDA Runtime function cudaGetDeviceCount provides a NULL value for symbolStatus when calling cuGetProcAddress, which leads to the segmentation fault in L604.
(gdb) bt
#0  cuGetProcAddress_v2 (symbol=0x7fffb30760b4 "cuInit", pfn=0x7fffb32a1d78, cudaVersion=2000, flags=0, symbolStatus=0x0) at hook.c:585
#1  0x00007fffb302294b in ?? () from /usr/local/cuda/lib64/libcudart.so.12
#2  0x00007fffb302e0cd in ?? () from /usr/local/cuda/lib64/libcudart.so.12
#3  0x00007fffb3031188 in ?? () from /usr/local/cuda/lib64/libcudart.so.12
#4  0x00007ffff7aa34df in __pthread_once_slow (once_control=0x7fffb32a1148, init_routine=0x7fffb3031140) at pthread_once.c:116
#5  0x00007fffb3073559 in ?? () from /usr/local/cuda/lib64/libcudart.so.12
#6  0x00007fffb30228d7 in ?? () from /usr/local/cuda/lib64/libcudart.so.12
#7  0x00007fffb3044681 in cudaGetDeviceCount () from /usr/local/cuda/lib64/libcudart.so.12
#8  0x00007fff689975d3 in c10::cuda::device_count_ensure_non_zero() () from /usr/local/lib/python3.8/dist-packages/torch/lib/libc10_cuda.so
#9  0x00007fff69821b91 in at::cuda::detail::CUDAHooks::initCUDA() const () from /usr/local/lib/python3.8/dist-packages/torch/lib/libtorch_cuda.so
#10 0x00007fffad14be6e in THCPModule_initExtension(_object*, _object*) () from /usr/local/lib/python3.8/dist-packages/torch/lib/libtorch_python.so
#11 0x00000000005c43c6 in ?? ()
#12 0x0000000000570b82 in _PyEval_EvalFrameDefault ()
#13 0x00000000005f5ee6 in _PyFunction_Vectorcall ()
#14 0x00000000006b3d06 in ?? ()
#15 0x00000000006b3f84 in PyObject_CallMethod ()
#16 0x00007fffad0e9e21 in torch::utils::cuda_lazy_init() () from /usr/local/lib/python3.8/dist-packages/torch/lib/libtorch_python.so
#17 0x00007fffaca81acd in torch::autograd::THPVariable_to(_object*, _object*, _object*) () from /usr/local/lib/python3.8/dist-packages/torch/lib/libtorch_python.so
#18 0x0000000000504449 in ?? ()
#19 0x000000000056bbe1 in _PyEval_EvalFrameDefault ()
#20 0x0000000000569d8a in _PyEval_EvalCodeWithName ()
#21 0x00000000005f60c3 in _PyFunction_Vectorcall ()
#22 0x000000000056bab6 in _PyEval_EvalFrameDefault ()
#23 0x000000000050b22e in ?? ()
#24 0x0000000000570b82 in _PyEval_EvalFrameDefault ()
#25 0x0000000000569d8a in _PyEval_EvalCodeWithName ()
#26 0x000000000050b3a0 in ?? ()
#27 0x0000000000570b82 in _PyEval_EvalFrameDefault ()
#28 0x0000000000569d8a in _PyEval_EvalCodeWithName ()
#29 0x000000000068e267 in PyEval_EvalCode ()
#30 0x000000000067d9b1 in ?? ()
#31 0x000000000067da2f in ?? ()
#32 0x000000000067dad1 in ?? ()
#33 0x000000000067fbf7 in PyRun_SimpleFileExFlags ()
#34 0x00000000006b8082 in Py_RunMain ()
#35 0x00000000006b840d in Py_BytesMain ()
#36 0x00007ffff7ad9083 in __libc_start_main (main=0x4ef3b0 <main>, argc=2, argv=0x7fffffffdba8, init=<optimized out>, fini=<optimized out>, rtld_fini=<optimized out>, stack_end=0x7fffffffdb98)
    at ../csu/libc-start.c:308
#37 0x00000000005faa2e in _start ()
(gdb) info args
symbol = 0x7fffb30760b4 "cuInit"
pfn = 0x7fffb32a1d78
cudaVersion = 2000
flags = 0
symbolStatus = 0x0

I'm using the ngc image nvcr.io/nvidia/pytorch:23.03-py3 for testing. It contains CUDA 12.1 / torch 2.0.0a0+1767026.
Through testing, I found that the original CUDA cuGetProcAddress will return NULL when it receives a NULL symbolStatus, and return a valid value when it receives a non-NULL symbolStatus.
So we can check if symbolStatus != NULL before attempting to set it, just like you commented above : )

I found that the original CUDA cuGetProcAddress will return NULL when it receives a NULL symbolStatus, and return a valid value when it receives a non-NULL symbolStatus.
Do you mean that cuGetProcAddress_v2(..., ..., ..., ..., NULL) will return a value of NULL?

Or that it will simply ignore the NULL symbolStatus argument?
I think the latter holds, as the upstream (part of CUDA Runtime API) cudaGetDeviceCount() calls it with a NULL argument, so it wouldn't make sense if cugetProcAdress_v2 returned NULL.
So I guess the latter holds.
In this case, here's what I suggest:
	/* This covers our custom "if" conditions.
	 * If we end up calling the real cuGetProcAddress_v2,
	 * it will overwrite this value.
	 */
	if (symbolStatus != NULL)
		*symbolStatus = CU_GET_PROC_ADDRESS_SUCCESS;

WDYT?

@grgalex Yes, it will simply ignore the NULL symbolStatus argument, but still return a valid CUresult.
So add a check if symbolStatus != NULL will solve this problem.

@pokerfaceSad Merged!
If you want make a sanity check on the current main, feel free to do so!


[SPARK-19714][DOCS] Clarify Bucketizer handling of invalid input

What changes were proposed in this pull request?
Clarify Bucketizer handleInvalid docs. Just a resubmit of https://github.com/apache/spark/pull/17169
How was this patch tested?
N/A

Test build #98685 has finished for PR 23003 at commit 0285326.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

Merged build finished. Test PASSed.

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/98685/
Test PASSed.


fix(connector-besu): Calls to contracts with parameters are failing

Describe the bug
When performing a call to a smart contract in order to read a value stored in the state, if the call contains parameters the operation will fail
To Reproduce
In this example calling getSingleStatus with the param id, the node will revert the transaction
const result = await this.connector.invokeContract({
  contractAbi: this.abi[environment.CONTRACT_NAME],
  contractAddress: environment.CONTRACT_ADDRESS,
  invocationType: EthContractInvocationType.CALL,
  methodName: "getSingleStatus",
  params: [id],
  web3SigningCredential: {
    ethAccount: environment.ACCOUNT_ADDRESS,
    type: Web3SigningCredentialType.PRIVATEKEYHEX,
    secret: environment.PRIVATE_KEY,
  },
});

Expected behavior
The value is returned

The parameters are passed in call() execution. That's for extra params (IE: sender, gas...) not the function params.
https://github.com/hyperledger/cactus/blob/add9cd94ed7fe4a7facda89749895f7b04de89a8/packages/cactus-plugin-ledger-connector-besu/src/main/typescript/plugin-ledger-connector-besu.ts#L201
The params are already specified in https://github.com/hyperledger/cactus/blob/add9cd94ed7fe4a7facda89749895f7b04de89a8/packages/cactus-plugin-ledger-connector-besu/src/main/typescript/plugin-ledger-connector-besu.ts#L198

Working on #564