plotting python numpy data that is noncontiguous and using implot stride option don't seem to work

[kuchi]

I noticed two potential issues: (or am just missing things)

1. It appears that the wrapping of the implot functions that take numpy arrays expect contiguous data. Would it be possible to enhance the wrapping to handle non-contiguous data. I believe the numpy C api allows that with a flag NPY_ARRAY_C_CONTIGUOUS. This may help users who may not be aware that they need to currently alway supply contiguous data.

2. The stride option in the plot functions does not seem to work as expected.

Sample code illustrating behavior:

import numpy as np
from imgui_bundle import imgui, immapp, implot

x = np.arange(0, np.pi * 5, 0.001)
y = np.sin(x)

def gui():
    if implot.begin_plot("###Plot"):
        
        implot.plot_line("curve np contiguous array", x, y)
        
        # Expectation is all lines below should look the same (with slight x offset to see them added) but just downsampled by 2x.
        
        # plot_line expects contiguous data 
        # Perhaps in the wrapper use np api to request always contiguous data  NPY_ARRAY_C_CONTIGUOUS
        #     https://numpy.org/devdocs/reference/c-api/array.html#c.NPY_ARRAY_C_CONTIGUOUS
        #     can also fix in python with np.ascontiguousarray(...)
        implot.plot_line("curve np sliced array", x[::2]+.1, y[::2]) 
        implot.plot_line("curve np contiguous sliced array", np.ascontiguousarray(x[::2]+.2), np.ascontiguousarray(y[::2])) 
        
        # Stride does not seem to work as expected. 
        implot.plot_line("curve contiguous with stride", x+.3, y, stride= 2)
        
        implot.end_plot()
immapp.run(gui, with_implot=True)

Example output

The red line with the stride option mangles the data:

The orange curve with a sliced numpy array which then non-contiguous reads incorrectly:

Thanks as aways for this great imgui wrapper bundle and the helloimgui ease of use library.

[pthom]

Hello,

As you can see below, the code to transform a numpy array to a full C-Style C Array (which ImPlot expects) is quite complex

imgui_bundle/external/implot/bindings/pybind_implot.cpp

Lines 714 to 767 in 5da9966

	m.def("plot_line",
	[](const char * label_id, const py::array & values, double xscale = 1, double xstart = 0, ImPlotLineFlags flags = 0, int offset = 0, int stride = -1)
	{
	auto PlotLine_adapt_c_buffers = [](const char * label_id, const py::array & values, double xscale = 1, double xstart = 0, ImPlotLineFlags flags = 0, int offset = 0, int stride = -1)
	{
	// convert py::array to C standard buffer (const)
	const void * values_from_pyarray = values.data();
	py::ssize_t values_count = values.shape()[0];
	// process stride default value (which was a sizeof in C++)
	int values_stride = stride;
	if (values_stride == -1)
	values_stride = (int)values.itemsize();
	#ifdef _WIN32
	using np_uint_l = uint32_t;
	using np_int_l = int32_t;
	#else
	using np_uint_l = uint64_t;
	using np_int_l = int64_t;
	#endif
	// call the correct template version by casting
	char values_type = values.dtype().char_();
	if (values_type == 'B')
	ImPlot::PlotLine(label_id, static_cast<const uint8_t *>(values_from_pyarray), static_cast<int>(values_count), xscale, xstart, flags, offset, values_stride);
	else if (values_type == 'b')
	ImPlot::PlotLine(label_id, static_cast<const int8_t *>(values_from_pyarray), static_cast<int>(values_count), xscale, xstart, flags, offset, values_stride);
	else if (values_type == 'H')
	ImPlot::PlotLine(label_id, static_cast<const uint16_t *>(values_from_pyarray), static_cast<int>(values_count), xscale, xstart, flags, offset, values_stride);
	else if (values_type == 'h')
	ImPlot::PlotLine(label_id, static_cast<const int16_t *>(values_from_pyarray), static_cast<int>(values_count), xscale, xstart, flags, offset, values_stride);
	else if (values_type == 'I')
	ImPlot::PlotLine(label_id, static_cast<const uint32_t *>(values_from_pyarray), static_cast<int>(values_count), xscale, xstart, flags, offset, values_stride);
	else if (values_type == 'i')
	ImPlot::PlotLine(label_id, static_cast<const int32_t *>(values_from_pyarray), static_cast<int>(values_count), xscale, xstart, flags, offset, values_stride);
	else if (values_type == 'L')
	ImPlot::PlotLine(label_id, static_cast<const np_uint_l *>(values_from_pyarray), static_cast<int>(values_count), xscale, xstart, flags, offset, values_stride);
	else if (values_type == 'l')
	ImPlot::PlotLine(label_id, static_cast<const np_int_l *>(values_from_pyarray), static_cast<int>(values_count), xscale, xstart, flags, offset, values_stride);
	else if (values_type == 'f')
	ImPlot::PlotLine(label_id, static_cast<const float *>(values_from_pyarray), static_cast<int>(values_count), xscale, xstart, flags, offset, values_stride);
	else if (values_type == 'd')
	ImPlot::PlotLine(label_id, static_cast<const double *>(values_from_pyarray), static_cast<int>(values_count), xscale, xstart, flags, offset, values_stride);
	else if (values_type == 'g')
	ImPlot::PlotLine(label_id, static_cast<const long double *>(values_from_pyarray), static_cast<int>(values_count), xscale, xstart, flags, offset, values_stride);
	else if (values_type == 'q')
	ImPlot::PlotLine(label_id, static_cast<const long long *>(values_from_pyarray), static_cast<int>(values_count), xscale, xstart, flags, offset, values_stride);
	// If we reach this point, the array type is not supported!
	else
	throw std::runtime_error(std::string("Bad array type ('") + values_type + "') for param values");
	};
	PlotLine_adapt_c_buffers(label_id, values, xscale, xstart, flags, offset, stride);
	}, py::arg("label_id"), py::arg("values"), py::arg("xscale") = 1, py::arg("xstart") = 0, py::arg("flags") = 0, py::arg("offset") = 0, py::arg("stride") = -1);

Concerning non contiguous arrays: I would advise you to use ascontiguousarray before calling implot, since I think it would be too complex to let the code handle another special case. However, I could raise a readable exception if if the received array is detected as not contiguous.

Concerning the stride option: I'm out of time to study this more in detail now. I'll give it a more complete look next week.

Cheers

[kuchi]

Since it is a lot of work. Perhaps for other users in the future there would be some info in the documentation to indicate that contiguous should only be used.

That is what I have been doing. I just figured some people may not understand why their plotting is not working so I thought I’d submit an issue.

Don’t worry about the stride option. I just subsample the numpy array and then use the as contiguous numpy function.

I didn’t want to cause any significant work. Thank you for looking at it.

[pthom]

Ok. Two modifications were applied.

1. check that arrays are contiguous and warn the user if not: e885a80
2. implot bindings: exclude params stride & offset : The way they were implemented depended on the size of the type in C which is not easy to access in Python. It was better to remove support for them altogether. 3714d5c

[Aman-Anas]

Doesn't the offset param simply depend on index from the user side? I have been able to use the offset param successfully for some plotting widgets, didn't experience any issues with it before. It's very useful for making "scrolling" graphs and such by offsetting an array like a circular buffer.

[pthom]

@Aman-Anas: Thanks for your remark and reactivity!

Indeed offset does not depend on the size of the type as long as stride is not used.

See extract from ImPlot code below:

template <typename T>
IMPLOT_INLINE T IndexData(const T* data, int idx, int count, int offset, int stride) {
    const int s = ((offset == 0) << 0) | ((stride == sizeof(T)) << 1);
    switch (s) {
        case 3 : return data[idx];
        case 2 : return data[(offset + idx) % count];
        case 1 : return *(const T*)(const void*)((const unsigned char*)data + (size_t)((idx) ) * stride);
        case 0 : return *(const T*)(const void*)((const unsigned char*)data + (size_t)((offset + idx) % count) * stride);
        default: return T(0);
    }
}

When stride is disabled, we only fall into case 2 and case 3 (which are OK). Case 0 and 1 do pointer arithmetic that depend on the size (IMHO (const unsigned char*)data + (size_t)((idx) ) * stride is a valid pointer iif stride is a multiple of sizeof(T))`

See commit 4316732 that restores the offset.