Is the current SigmoidTransform implementation correct?

1

The log_abs_det_jacobian in the SigmoidTransform, is implemented as

-(y.reciprocal() + (1 - y).reciprocal()).log()

but I think the correct implementation should be

-(y.reciprocal().log() + (1 - y).reciprocal().log())

or even simpler y.log() + (1-y).log() since log(a) + log(b) does not equal to log(a + b).

1 Like
2

Hi Xiucheng, I agree it’s a bit confusing although the log_abs_det_jacobian is correct in the current implementation.

The function log_abs_det_jacobian calculates log(|dy/dx|) where y is the output and x is the input. Now, this is equal to -log(|dx/dy|), which is closer to the formula log_abs_det_jacobian uses in this implementation for the sigmoid transform.

For the sigmoid transform we have, dy/dx = sigma(x) * (1 - sigma(x)) = y(1-y). Then the function should return log(y*(1-y)). If we multiple and divide by -1 we get that it’s equal to -log((y*(1-y))^-1). Expanding the fraction into two terms gives, -log(1/y + (1/(1-y))) which is what is calculated in the code.

Alternative, you could start from dx/dy and x = logit(y) to get the same result. Hope this helps!

1 Like
3

Hi @stefanwebb. Thanks so much for your step by step analysis. But expanding -log((y*(1-y))^-1) into two terms, should not it be -log(1/y) - log(1/(1-y))?

4

The fraction is expanded inside the log to give -log(1/y + (1/(1-y))) using 1/(y*(1-y)) = 1/y + 1/(1-y). Expanding the log into two terms gives an equivalent expression

5

Oh, I see. In this case, log(1/y + (1/(1-y))) actually is equivalent to log(1/y) + log(1/(1-y)).

1 Like