Expose alpha and theta type for parametrized activations

[jmitrevs]

Description

The type of the parameter for parametrized activations were not configurable before, being set the same as either the input of result. Generally there is no reason for that requirement, so this makes the type explicitly configurable,

Type of change

• Bug fix (non-breaking change that fixes an issue)
• New feature (non-breaking change which adds functionality)

Tests

The standard activations tests (slightly modified) provide the test.

Checklist

• I have read the guidelines for contributing.
• I have commented my code, particularly in hard-to-understand areas.
• I have made corresponding changes to the documentation.
• My changes generate no new warnings.
• I have installed and run pre-commit on the files I edited or added.
• I have added tests that prove my fix is effective or that my feature works.

[jmitrevs]

Note: this PR is needed to fix test_qonnx.py::test_sep_conv[Vitis] in #979.

[vloncar]

Why not ELU while we're at it?

[jmitrevs]

I left then to be the default values, though you can configure them otherwise manually. Is there a better choice?

[vloncar]

I agree with that, my comment was mostly that the comment in the code suggests this is only applicable to thresholded relu while it actually works for three layers (leaky, threholded and elu)

[jmitrevs]

I can modify the comment saying that the others are left to the default precision.

[vloncar]

I think that the idea of configurable threshold is good in priciple. However I think this way we risk of having an inconsistency between parametrized activations. Thresholded relu gets to have the same type as input, which is the current behavior anyway, but leaky and elu get the default (which may be different from what is the input to the current layer), and prelu is not even considered so it gets the default that way. Why not handle all of them to have a consistent behavior (whatever that is, may be input precision for continuity, may be default precision)? Also, why use input precision when we can be smart and figure out the required precision given that you have access to the activ_param to figure out exactly how many bits you need to represent that number/array.

[jmitrevs]

They have a fundamentally different function. Threshold compares the input values to a threshold, so it makes sense for it to be the same type as the input. One can tune it if preferred, but there is an implicit connection with the input type. If the input type logically would have units, the threshold would have the same units. The others are scaling factors, so they have no connection to the input type at all. It just doesn't make sense to make them the same as the input type. There is a fundamental difference. I did consider adding prelu to the match on line 87, but then _infer_par_act_precision would just ignore it and have it set the default anyway, so I didn't add it to the match. But I am not fundamentally opposed to that.

[jmitrevs]

I think logically, for threshold comparisons the same type makes sense, so it seems like a good setting, I believe better than the default. For scaling factors, we have no guidance, so the default precision makes sense.

[jmitrevs]

But the suggestion about looking at the activ_param is interesting. I have to think about it. One can set the range, but not necessarily the number of bits.

[vloncar]

For a scale (and threshold) that is a scalar the guidance is simple, it's a number for which we can figure out the best precision to store (without using too many decimal bits to match the original stored in float). Does get trickier for the case of PReLU where we would need a smarter way to decide what precision suits best for the most of the array. I undestand your logic for the current behavior. I'm fine with merging this as-is and do a "smarter" way as a follow-up

[jmitrevs]

Actually, I think all activations can be updated. The output type can be unsigned for relu (but otherwise matching the input type), restricted in range for sigmoid and tanh, etc. It may be good to have another pull request that does precision propagation for all activations.

[vloncar]

Technically, this got in by accident from the QONNX PR

[jmitrevs]

It was not supposed to include that--I thought I was being careful. I had updated example-models in my work area. I am not sure if they are the final ones, either. I can try to recreate this without the example-models change if you prefer.

[vloncar]

I think it is fine to remain. We'll update that pointer anyway.