Issues when use_cell_type is not None

[AprilYuge]

Hi,

I found some code are missing for supervised/semi-supervised version of scGLUE when use_cell_type is given.

1. The Classifier in sc.py is missing (line 480). My modification: class Classifier(torch.nn.Module):

     r"""
     Linear label classifier
   
     Parameters
     ----------
     n_hidden,
         Input dimensionality
     num_of_class
         Output dimensionality
     """
   
     def __init__(self, n_hidden, num_of_class):
         super(Classifier, self).__init__()
         self.cell = torch.nn.Sequential(
             torch.nn.Linear(n_hidden, num_of_class)
         )
   
     def forward(self, embedding):
         cell_prediction = self.cell(embedding)
         return cell_prediction

2. The parameters of self.net.u2c are not included in the self.vae_opt in glue.py (line 321-328). My modification:

    if self.net.u2c:
        self.vae_optim = getattr(torch.optim, optim)(
            itertools.chain(
                self.net.g2v.parameters(),
                self.net.v2g.parameters(),
                self.net.x2u.parameters(),
                self.net.u2x.parameters(),
                self.net.u2c.parameters()
            ), lr=self.lr, **kwargs
        )
       else:
           self.vae_optim = getattr(torch.optim, optim)(
               itertools.chain(
                   self.net.g2v.parameters(),
                   self.net.v2g.parameters(),
                   self.net.x2u.parameters(),
                   self.net.u2x.parameters()
               ), lr=self.lr, **kwargs
           )

Best,

Yuge

[Jeff1995]

Hi Yuge! Thanks for the report.

I'm afraid the first fix is not necessary, as Classifier directly inherits from torch.nn.Linear, it will work just fine.

The second problem is valid though and the fix is correct. However, scglue.models.glue.GLUE does not define u2c, only scglue.models.scglue.SCGLUE does, so I guess it makes more sense to put the fix in scglue.models.scglue.SCGLUETrainer (scglue.py:L205). Also, the same issue exists for SCCLUE (scclue.py:L600). Do you wish to create a pull request for that? Or if you're busy with other work I can fix it myself.

Thanks again for the report and proposed fixes! These are important issues.

[AprilYuge]

Hi Zhijie,

Thanks for your reply! You are right that Classifier should be fine by inherting from torch.nn.Linear. For the second point, I modified inside GLUETrainer instead of scGLUETrainer because that's where self.vae_optim is defined, but we can also overwrite that inside scGLUETrainer if self.net.u2c is not None.

I can submit a pull request these days.

Best,

Yuge

[Jeff1995]

Well that's great! I'd prefer the fix inside scglue.py and scclue.py, and preserve glue.py as a reference implementation of the framework that does not involve supervised classification. Let me know when you have the PR ready.

[AprilYuge]

Hi Zhijie,

Sorry for the late reply. I just submitted a PR for this issue. Take a look when you have time.

Thanks,

Yuge

[Jeff1995]

Thank you for the contribution! I have merged the PR.