positional encodings

[amikki]

Hi @wehos,
I really like your work and how you organized the codebase 🙂
I have a question about the positional encodings.
I noticed that in the code, the PE are added to the xy coordinates, but it = 0, so I cannot see how the model learned about the cell-cell interactions.

Thanks,
Makky

[wehos]

Hi Makky,

Thanks for your interest! I think you are referring to the same issue here and this is indeed a good catch. #19 (comment)

[amikki]

Hi @wehos,

Thank you so much for your reply.
I have two relevant questions:
1- What does the parameter 'order_required' in the fit function refer to?
2- In my understanding transformers learn the spatial information from the positional encodings which is zero as we discussed before, so how did CellPLM learn the spatial information or it actually did not learn?
Thank you so much in advance!
Makky

[wehos]

(1) I did not see "order_required" in fit functions' parameters. If you are referring to the inference function as below,

CellPLM/CellPLM/pipeline/imputation.py

Line 36 in b59ee76

def inference(model, dataloader, split, device, batch_size, order_required=False):

If the cell order is not asked to be preserved, to save time, the model will only infer on specified splits, e.g., validation or test. The returned prediction will not cover all the cells, and the order of predictions is not guaranteed. In this case, we mostly care about the loss instead of the prediction. If we also need the prediction, the dataset split will be ignored and all the cells will be predicted and returned in the original order.

(2) All the codes in this github repo were uploaded at a certain point after we finished our manuscript. I have checked on my private github repo and noticed that the problematic codes were modified on Sep 15, 2023 (our manuscript was first submitted to NeurIPS in May 2023). I believe this change was made when I fine-tuned the model for scRNA-seq data. For now, feel free to remove the comment on spatial transcriptomics data.

Changelog from my private repo (the link is for reference purpose but is not public available):

- pe = self.pe_enc(pe_input)
+ pe = 0.#self.pe_enc(pe_input)
https://github.com/wehos/CellBert/commit/9fc5b1a7bf7b20aa9eac95beba6de067f2684619#diff-286a4c753e046ce11d844e262d909e0ae39ee1fa09b63e02d3be2371cbe78efbL78

[Hosseini7798]

Hi @wehos,
Thanks for this intuitive project!
I noticed a small issue in Sinusoidal2dPE. The line that converts 2D coordinates (x, y) to a 1D index currently reads:

pe_input = x * self.width + y

Should'nt it be:

pe_input = y * self.width + x

Since x is in the range of width and y in the range of height, this adjustment ensures correct indexing after flattening.