WIP implement #211: native `min` and `max` over arrays via `glm::min` / `glm::max` #213

cspotcode · 2023-02-26T19:52:42Z

Fixes #211

I think I found where this needs to be added in the code, but it would be way faster for you to point out what exactly I should be copying to generate the correct switch-case block to call all the templated implementations of apply_min. I see that apply_min takes a std::vec and I'm pretty sure that glmArray is not a std::vec. So does this need a new apply_min_of_glmArray template?

Zuzu-Typ · 2023-02-26T20:15:42Z

Unfortunately, this is a bit more complex than it might seem at first.

If the array has elements of a number type (e.g. 32-bit floating point values), it's trivial to perform a min or max operation on them. But what if the elements are vectors, quaternions or matrices?
Since you can't define a logical "less than" operation for vectors (for example is vec2(1, 0) < vec2(0, 1) or vec2(0, 1) < vec2(1, 0)?), you would have to follow glm's way, which is calculating the minimum value of each component of the vector. This however isn't exactly trivial to implement.
I'm very happy for your support and that you're trying to find a solution for the issue, but I'd rather save you the headache of trying to understand the enormous amount of code I have assembled over the years (which might be required to implement the min and max functions).

cspotcode · 2023-02-27T04:37:45Z

you would have to follow glm's way, which is calculating the minimum value of each component of the vector. This however isn't exactly trivial to implement.

I was imagining that, if PyGLM already has an implementation of min(vec2, vec2) then I would piggy-back on that implementation. Is that a bad approach?

Zuzu-Typ · 2023-02-27T08:31:20Z

I was imagining that, if PyGLM already has an implementation of min(vec2, vec2) then I would piggy-back on that implementation. Is that a bad approach?

That would be a good idea ~~in theory, but quite inefficient. You'd have to call the function for every element (except for the first) in the array. E.g. for an array with 5 components: min(min(min(min(e1, e2), e3), e4), e5)~~

A better approach would be to split the array into components if it is a vec, quat or mat array, call the min function with each one of those components and re-assemble the result array. I have some ideas to make this a little more performance efficient (e.g. by using multithreading)

Well, actually I was wrong. It is probably the most efficient way. That is, using glm::min. You wouldn't want to use the apply_min or min_ functions provided by PyGLM though, because it would be a significant overhead.

cspotcode · 2023-02-28T20:10:44Z

Ok thanks, I see that the code has switch-case blocks where you check the type of something, then call the correct templated implementation. I think I need to do the same here but call a templated implementation of glm::min. Is that correct?

PyGLM/PyGLM/functions/detail/func_common.h

Lines 239 to 263 in 9f2cdfe

    
           #define min_GEN_TYPE_SWITCH_STATEMENT_FOR_VECTOR(L) \ 
        
           switch (pti->format) { \ 
        
           case get_format_specifier<float>(): \ 
        
           	return apply_min_from_PyObject_vector_vector<L, float>(items); \ 
        
           case get_format_specifier<double>(): \ 
        
           	return apply_min_from_PyObject_vector_vector<L, double>(items); \ 
        
           case get_format_specifier<int32>(): \ 
        
           	return apply_min_from_PyObject_vector_vector<L, int32>(items); \ 
        
           case get_format_specifier<uint32>(): \ 
        
           	return apply_min_from_PyObject_vector_vector<L, uint32>(items); \ 
        
           case get_format_specifier<int64>(): \ 
        
           	return apply_min_from_PyObject_vector_vector<L, int64>(items); \ 
        
           case get_format_specifier<uint64>(): \ 
        
           	return apply_min_from_PyObject_vector_vector<L, uint64>(items); \ 
        
           case get_format_specifier<int16>(): \ 
        
           	return apply_min_from_PyObject_vector_vector<L, int16>(items); \ 
        
           case get_format_specifier<uint16>(): \ 
        
           	return apply_min_from_PyObject_vector_vector<L, uint16>(items); \ 
        
           case get_format_specifier<int8>(): \ 
        
           	return apply_min_from_PyObject_vector_vector<L, int8>(items); \ 
        
           case get_format_specifier<uint8>(): \ 
        
           	return apply_min_from_PyObject_vector_vector<L, uint8>(items); \ 
        
           case get_format_specifier<bool>(): \ 
        
           	return apply_min_from_PyObject_vector_vector<L, bool>(items); \ 
        
           }

What fields should I be checking on the array to switch-case on? Which fields on glmArray indicate the shape of the contents, and is there an example of a reinterpret_cast on glmArray.data that will let me iterate it as a C array of vec2?

PyGLM/PyGLM/types/types.h

Lines 291 to 303 in 9f2cdfe

    
           struct glmArray { 
        
           	PyObject_HEAD 
        
           		char format; 
        
           	uint8 shape[2]; 
        
           	uint8 glmType; 
        
           	Py_ssize_t nBytes; 
        
           	Py_ssize_t itemCount; 
        
           	Py_ssize_t dtSize; 
        
           	Py_ssize_t itemSize; 
        
           	PyTypeObject* subtype; 
        
           	PyObject* reference; 
        
           	bool readonly; 
        
           	void* data;

Does glmArray.shape store the dimensions of the contents, so shape = 2,1 means it stores vec2s? To check the type of the numbers -- float, int, etc -- should I be looking at glmType or format or subtype?

cspotcode · 2023-03-01T16:51:44Z

❯ hyperfine --show-output --runs 5 --parameter-list impl py,glm 'python ./test_min_{impl}.py'
Benchmark #1: python ./test_min_py.py
  Time (mean ± σ):     10.412 s ±  0.158 s    [User: 10.408 s, System: 0.004 s]
  Range (min … max):   10.169 s … 10.590 s    5 runs
 
Benchmark #2: python ./test_min_glm.py
  Time (mean ± σ):     880.9 ms ±  22.3 ms    [User: 876.6 ms, System: 4.0 ms]
  Range (min … max):   854.0 ms … 900.1 ms    5 runs
 
Summary
  'python ./test_min_glm.py' ran
   11.82 ± 0.35 times faster than 'python ./test_min_py.py'

cspotcode · 2023-03-02T22:28:43Z

This'll need a lot more macro work to support all possible data types that you can store in a pyglm array, but I have it prototyped for arrays of floats and vec2s. Benchmark shows it's definitely much faster.

broken attempt at this; need guidance

b3dc39c

cspotcode marked this pull request as draft February 26, 2023 19:52

cspotcode added 2 commits February 28, 2023 15:30

WIP

36b615a

proof-of-concept, very messy, only works for arrays of floats

a91baad

cspotcode force-pushed the fix-211 branch from 375a120 to a91baad Compare March 1, 2023 16:51

cspotcode added 2 commits March 1, 2023 17:08

Merge remote-tracking branch 'upstream/master' into fix-211

fff6c28

WIP

5695087

cspotcode changed the title ~~Fix #211: broken attempt at this; need guidance~~ WIP implement #211: native min and max over arrays via glm::min / glm::max Mar 2, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

WIP implement #211: native `min` and `max` over arrays via `glm::min` / `glm::max` #213

WIP implement #211: native `min` and `max` over arrays via `glm::min` / `glm::max` #213

cspotcode commented Feb 26, 2023 •

edited

Zuzu-Typ commented Feb 26, 2023

cspotcode commented Feb 27, 2023

Zuzu-Typ commented Feb 27, 2023 •

edited

cspotcode commented Feb 28, 2023

cspotcode commented Mar 1, 2023

cspotcode commented Mar 2, 2023

WIP implement #211: native min and max over arrays via glm::min / glm::max #213

Are you sure you want to change the base?

WIP implement #211: native min and max over arrays via glm::min / glm::max #213

Conversation

cspotcode commented Feb 26, 2023 • edited

Zuzu-Typ commented Feb 26, 2023

cspotcode commented Feb 27, 2023

Zuzu-Typ commented Feb 27, 2023 • edited

cspotcode commented Feb 28, 2023

cspotcode commented Mar 1, 2023

cspotcode commented Mar 2, 2023

WIP implement #211: native `min` and `max` over arrays via `glm::min` / `glm::max` #213

WIP implement #211: native `min` and `max` over arrays via `glm::min` / `glm::max` #213

cspotcode commented Feb 26, 2023 •

edited

Zuzu-Typ commented Feb 27, 2023 •

edited