s175573 / GIANA

Ultrafast TCR clustering algorithm based on geometric isometry
Other
63 stars 30 forks source link

Clarification on inconsistency between implementation and GIANA paper #3

Closed jdaymude closed 2 years ago

jdaymude commented 2 years ago

I've been working my way through the details of GIANA's implementation and comparing it to the Methods section of the Nature Communications paper. In the "GIANA method description" subsection, Eq. 8 defines $\Omega_2$ as:

0, I
I, 0

where 0 and I are the r-dimensional square zero and identity matrices, respectively. Eq. 12 then defines $\Omega_3$ as:

0, 0, I
I, 0, 0
0, I, 0

where again 0 and I are the r-dimensional square zero and identity matrices, respectively. Finally, Eq. 13 defines $\Omega_6$ as:

0, 0, \Omega_2
\Omega_2, 0, 0
\0, \Omega_2, 0

which is produced by replacing all instances of I in $\Omega_3$ with \Omega_2 and extending the dimension of the zero matrices to 2r.

However, the implementation of GIANA (both version 4 and 4.1) implement $\Omega_6$, or M6 in the code, as:

M6=np.concatenate((np.concatenate((ZERO45,M0),axis=1),np.concatenate((M0, ZERO45),axis=1)))

GIANA4.py, Line 103

This implementation replaces all instances of I in $\Omega_2$ with \Omega_3 (M0 in the code), which is exactly the opposite of what is done in the paper. Moreover, these are not the same matrices. Which version is right? (And which were the results of the Nature Communications paper performed with?)

s175573 commented 2 years ago

Hi Josh,

Thank you for catching this. The matrix in the code was used to generate the results in the paper. I believe this won’t cause any difference as it is also an element in the 6-th order cyclic group.

Best, Bo

From: Josh Daymude @.> Date: Thursday, June 30, 2022 at 7:16 PM To: s175573/GIANA @.> Cc: Subscribed @.***> Subject: [s175573/GIANA] Clarification on inconsistency between implementation and GIANA paper (Issue #3) EXTERNAL MAIL

I've been working my way through the details of GIANA's implementation and comparing it to the Methods section of the Nature Communications paper.

In the "GIANA method description" subsection, Eq. 8 defines $\Omega_2$ as:

0, I

I, 0

where 0 and I are the r-dimensional square zero and identity matrices, respectively. Eq. 12 then defines $\Omega_3$ as:

0, 0, I

I, 0, 0

0, I, 0

where again 0 and I are the r-dimensional square zero and identity matrices, respectively. Finally, Eq. 13 defines $\Omega_6$ as:

0, 0, \Omega_2

\Omega_2, 0, 0

\0, \Omega_2, 0

which is produced by replacing all instances of I in $\Omega_3$ with \Omega_2.

However, the implementation of GIANA (both version 4 and 4.1) implement $\Omega_6$, or M6 in the code, as:

M6=np.concatenate((np.concatenate((ZERO45,M0),axis=1),np.concatenate((M0, ZERO45),axis=1)))

— GIANA4.py, Line 103https://github.com/s175573/GIANA/blob/642f5c9551a10edf44aba03280a723bd966e1808/GIANA4.py#L103

This implementation replaces all instances of I in $\Omega_2$ with \Omega_3 (M0 in the code), which is exactly the opposite of what is done in the paper. Moreover, these are not the same matrices. Which version is right? (And which were the results of the Nature Communications paper performed with?)

— Reply to this email directly, view it on GitHubhttps://github.com/s175573/GIANA/issues/3, or unsubscribehttps://github.com/notifications/unsubscribe-auth/AKWYQC2DYC2UWULA532PKDTVRY2G3ANCNFSM52LBF4DQ. You are receiving this because you are subscribed to this thread.Message ID: @.***> CAUTION: This email originated from outside UTSW. Please be cautious of links or attachments, and validate the sender's email address before replying.


UT Southwestern

Medical Center

The future of medicine, today.

jdaymude commented 2 years ago

Thanks very much for the clarification; closing the issue.