Closed mudphudwang closed 11 months ago
Thanks for flagging, will look into this/try to repro and then get back!
Thanks very much again for flagging this. We updated the loss implementation in PR https://github.com/AdaptiveMotorControlLab/CEBRA/pull/86, including numerical tests against a reference implementation.
In short, we changed the broadcasting, but this has little effect on the performance. Concretely, when testing the 0.2.0 version (old implementation) vs. the 0.3.0rc2 (new implementation) version, we find no significant differences.
Here, for the synthetic benchmarking:
scipy.stats.ttest_rel(new_scores, old_scores)
TtestResult(statistic=1.4435649958049843, pvalue=0.15201838297772313, df=99)
As shown here (note the Y-axis range):
Is there an existing issue for this?
Bug description
Congratulations on this great project and publication!
I was browsing the code and noticed a potential issue with
cebra.models.criterion.infonce
. I assume thatc
in the function is just there for numerical stability oflogsumexp
, and that the function is supposed to return$L = \mathbb{E}_x [-\phi(x_i, y^{+}_i) + \log \sum_{j=1}^{n} e^{\phi(x_i, y^{-}_{ij})}]$
? If so, then I think that there might be an error in howc
is being broadcasted withneg_dist
, which makes the function return incorrect values.Operating System
Ubuntu 18.04
CEBRA version
0.2.0
Device type
Core i9 / RTX 3090
Steps To Reproduce
Relevant log output
Anything else?
No response
Code of Conduct