also, how can we be sure that inputs[1]is not 0 ? I feel like 1 / inputs[1] can fail here no ?

btw are values always positive ? or are we talking about any floats (including negatives) ?

I do this right before:

            min_non_zero_value = numpy.min(numpy.abs(inputs[1]))

            # mypy
            assert min_non_zero_value is not None and min_non_zero_value > 0

should be good no?

ah yes right, I missed the assert. But does that mean that we always expect values to be strictly positive ? how can we ensure that ?

What I could check is that no value are quant dequant to a 0 maybe

yes but my question is more about "how can you be sure of that ?" !

you are right we do the abs, so my questions is then : how can we be sure that we never get 0 in inputs[1] ? I understand that we assert or could add more assert, but I don't get how we are sure that this/these assert(s) will never ever fail in any circumstances (and if we can't, that's an issue imo) !

We can't. If the user provides a 0 it will fail. As any division fails when there are 0 in the denominator.

ah I see, in that case if it's a user issue, then we should have a raise ValueError with an explicit message rather than an empty assert. Unless an error is triggered before reaching this point (and in that case, is it explicit enough ?)

The assert here is for mypy. Not sure we should rewrite basic error message such as division by 0.

well I won't push too much on this one, but I just fear that the error will not be a clear "DivisionByZeroError" and might confuse the user, would be worth a check or even a test

how is this working ? just replacing 0 with the (float) min ? I imagine that means that values are expected to be positive ! And then still, should we instead add self.min_non_zero_value to ìnput_1`, instead of replacing 0, right ?

in any case we should explain in comments how all this work

Hmm yeah I don't understand how that work either. I did that to fix the calibration but it should fail when compiling the circuit. Unless the numpy.where never does anything? I need to change that indeed thanks.

so what should we do here then ?

Actually, here we are within a PBS and we assign values that are 0 to self.min_non_zero_value. That should work fine within a PBS.

yes but my observation was that, since we replace all zeros with self.min_non_zero_value :

• since this value is computed through numpy.min(numpy.abs(inputs[1])) , does that mean we expect input_1 to always be positive ?
  • if not, for example, [-2, 4, 0] would be replaced by [-2, 4, 2] and I don't think it makes much sense
  • if so, how can we be sure that input_1 is >= 0 ? more that having asserts I mean (like my comment in https://github.com/zama-ai/concrete-ml/pull/690/files#r1677747936)
• also, aren't we breaking the values' distribution by doing something like this ? initially I suggested to instead do something like input_1 += min_non_zero_value (or input_1 += if positive) but don't think that's desirable as well. What about requantizing on [1, max_val] ? Anyway, I feel just replacing 0 by this new val is not exactly right

I totally understand your worry on this however! Let's find the best solution.

oh ok sorry I don't know why I kept seeing argmax and not argmin, so yeah ok we just get the closest value to 0, makes much more sense indeed

would it make sense to instead add an epsilon to this 0 ? or simply to all values ? not sure it'll change much since we re-quantize after the 1/x

apart from that, I'm not sure I have better ideas here . If you want we could discuss this tomorrows yes

would it make sense to instead add an epsilon to this 0 ?

Yes that's what I proposed. An epsilon would make sense if it's represented by the quantized values. The scale is basically the smallest represented floating point. So we could use this instead of the min(abs(x)). But I am not entirely sure we can get the actual quantizer of the input. Also I am afraid that for large bit-width this epsilon is going to be super small and will lead to numerical errors.

yeah numerical errors could be an issue indeed. But actually this epsilon should be added to all values not just the 0 maybe ? in order to keep the same distribution ? not sure how the quant params will handle this though

are we sure about this change ?

We were running that check twice.

not sure I see where we were doing the second chck 🤔

check_r2_score(raw_output_vv, quantized_output_vv) is right below the if

but you added that one 😅 , my question was about the if statement that you added, like mentioned below

Ah yeah what am saying -_-'. Sorry. So yes the problem is as I said, correctness were fine before with univariate LUT but in QuantizedDiv|Mul we have quant requant which impact the final accuracy.

do we know by how much it can impact ? how "strong" do we expect this impact to be ? I'm just being a bit suspicious here because of things like https://github.com/zama-ai/concrete-ml/pull/690/files#r1634728461 or https://github.com/zama-ai/concrete-ml/pull/690/files#r1634723604 😅