First change: when orthogonalizing, use abs of the orthogonalized data rather than just (data * data_conj_scaled).imag). Not 100% sure this is right but conceptually the thing we're correlating with is abs(data) so it seems to make sense to take the abs here, too...

I think the original operated on np.log10(label_data_orth ** 2) so an abs was implicitly taken by the squaring operation.

I am not sure about the log10, but they do square and take the log-transform:
Citation from their supplementary information:
"The signal Y(t,f) is orthogonalized in the complex plane with respect to X(t,f). This results in a positive number |Y_orth_X(t,f)| which is then squared and log-transformed, and correlated with [|X(t,f)|." Source

log10 is the same as log up to a scale factor, and that scale factor goes away in the correlation so it doesn't really matter. But np.log is faster than np.log10 so why not!

... but that text does make it clear that it should result in a positive number even before the square and log transform, so I think this is correct now (and will continue to be on the next push where I change np.log10 to np.log)

I think abs was missing in the original code, because of the squaring in the next step.

Second change is what @Avoide talked about in the other issue, we need to correlate with the correct pairs. This (plus the log=True, absolute=False) makes our code match both what @Avoide suggested and what FieldTrip produces.

Second change is what @Avoide talked about in the other issue, we need to correlate with the correct pairs. This (plus the log=True, absolute=False) makes our code match both what @Avoide suggested and what FieldTrip produces.

We were using np.corrcoef but this is much faster

Perhaps consider adding a comment for Python newcomers that this updates the diagonal efficiently.

I always had a feeling since our working session at Biomag2018 that the MNE implementation does something different than the prototype.

@dengemann IIRC the code here was the prototype that I used to code a more efficient version in MNE, so it should have replicated it properly. Unfortunately I had to change this prototype code to do something different in order to get it to follow FieldTrip and the paper definitions...

No problem, If we can come up with a more flexible set of options this is good for research on this method.
The comparisons in the previous discussion suggest that difference may be rather subtle.

I second, differences will be subtle and additional options will be worth comparing.

@SherazKhan the actual critical change in the code below is from data_mag_nomean[li] which only used the lith label's data to correlate with all N labels label_data_orth series, and this code now uses data_mag_nomean, i.e., all N labels data to correlate with all N labels label_data_orth series.

It's probably easier to see if you look at the "simplified" version here. The old/master version of the code does the correlation with the jj original (magnitude) data that you could call y_mag:

                corr[ii, jj] += np.abs(np.corrcoef(np.abs(epoch_data[jj]), y_orth_x)[0, 1])

Instead of this correlation, with (magnitude) of the iith data that is called x_mag here, which is now implemented in this PR, equivalently written as:

                corr[ii, jj] += np.abs(np.corrcoef(np.abs(epoch_data[ii]), y_orth_x)[0, 1])

@larsoner indeed, this is more closer to the original Hipp version.