LinearCombination: higher-dimension capable LLVM implementation#3533
LinearCombination: higher-dimension capable LLVM implementation#3533kmantel wants to merge 1 commit into
Conversation
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| expected = np.prod(tmp, axis=0) * scale + offset | ||
|
|
||
| # wider tolerances needed for fp32 | ||
| np.testing.assert_allclose(res, expected, rtol=3e-5, atol=2e-7) |
|
This PR causes the following changes to the html docs (ubuntu-latest-3.11): See CI logs for the full diff. |
supports parameter specifications as scalar, len-1 vector, or N-dim array matching shape of input (weights, exponents) or output (scale, offset) (len-M vector elementwise for (M, ...) shape arrays is intended to work, but does not work in non-compiled PNL)
|
This PR causes the following changes to the html docs (ubuntu-latest-3.11): See CI logs for the full diff. |
| self._gen_llvm_combine(builder, ctx=ctx, vi=arg_in, vo=arg_out, params=params) | ||
| return builder | ||
|
|
||
| def _llvm_combine_body(self, builder, ctx, vi, vo, val_f, pow_f, comb_op, params, indices): |
There was a problem hiding this comment.
All code generating functions should be prefixed with _gen_llvm
| if len(param_type) == 1 and not isinstance(param_type.element, pnlvm.ir.ArrayType): | ||
| index = [ctx.int32_ty(0)] | ||
|
|
||
| if not isinstance(index, list): |
There was a problem hiding this comment.
It might be better to guarantee that index/indices is always a list.
There was a problem hiding this comment.
I did that trying to deal with using lists as index in LinearCombination (ex L1568 exponent = self._gen_llvm_load_param(ctx, b, params, EXPONENTS, in_idx, 1.0) without changing other TransformFunctions or overriding _gen_llvm_load_param and couldn't think of something better. I'm fine changing it if it's better to do one of those or something else I haven't thought of.
| offset = self._gen_llvm_load_param(ctx, builder, params, OFFSET, index, -0.0) | ||
| def _gen_llvm_function_body(self, ctx, builder, params, _, arg_in, arg_out, *, tags: frozenset): | ||
| # Sometimes we arg_out to 2d array | ||
| if self.defaults.variable.ndim == self.defaults.value.ndim: |
There was a problem hiding this comment.
It'd be better to use compiled data types rather than python dimensionality to maintain a single source of truth.
|
|
||
| if isinstance(ptro.type.pointee, pnlvm.ir.ArrayType): | ||
| with pnlvm.helpers.array_ptr_loop(builder, ptro, f"combine_axis{len(indices)}") as (b, idx): | ||
| self._llvm_combine_body(b, ctx, vi, vo, val_f, pow_f, comb_op, params, [*indices, idx]) |
There was a problem hiding this comment.
Rather that recursing here, it might be clearer to use recursive_iterate_arrays over the output array.
There was a problem hiding this comment.
Can this be used when exponents/weights are either scalar or matching the input shape? Not sure about it, but it seems that since recursive_iterate_arrays yields gep results, it would need to conditionally include or exclude those parameter vals in the recursive_iterate_arrays calls, and need to be repeated per combination.
| # random 1/-1 | ||
| weights = 2 * (np.round(RANDh_V['weights'][variable.shape]) - .5) | ||
| if exponents == 'A': | ||
| exponents = RANDh_V['exponents'][variable.shape] |
There was a problem hiding this comment.
Why can't the above be passed as parametrized arguments?
There was a problem hiding this comment.
They could be generated in the test too.
Otherwise, I think the issue is that they need to match [parts of] the variable shape, but AFAIK there isn't a way to couple parametrization vars (as in, only do (var1,exp1), (var2,exp2)... and not also (var1,exp2), (var2,exp1)), so I'd still need to pass some function or dict to get the correctly shape value out.
But yeah I can also pre-calculate these a bit better.
3 participants
supports parameter specifications as scalar, len-1 vector, or N-dim array matching shape of input (weights, exponents) or output (scale, offset)
(len-M vector elementwise for (M, ...) shape arrays is intended to work, but does not work in non-compiled PNL)