Post-SMTB cleanup

[ilsenatorov]

Most important changes:

• Added unit testing with pytest
• Added github action to run the tests
• Cleaned up redundant code
• Rearranged some files here and there
• Improved config/training logic for finetuning

[Old-Shatterhand]

Applies to all files: Comment the code. It will be a project with >5 people working on and maintaining it, we need to set some documentation standards to not get completely lost before christmas.

[ilsenatorov]

Applies to all files: Comment the code. It will be a project with >5 people working on and maintaining it, we need to set some documentation standards to not get completely lost before christmas.

I would suggest comments/docstring on code that is not self-explanatory. For example:

def setup(self, stage: str | None = None):
        """Create train, val, test datasets."""
        self.train = DownstreamDataset(self.data_dir / "train", self.layer_num)
        self.valid = DownstreamDataset(self.data_dir / "valid", self.layer_num)
        self.test = DownstreamDataset(self.data_dir / "test", self.layer_num)

    def _get_dataloader(self, dataset: DownstreamDataset, shuffle: bool = False) -> torch.utils.data.DataLoader:
        """Create a DataLoader for a given dataset."""
        return DataLoader(
            dataset, batch_size=self.batch_size, num_workers=self.num_workers, shuffle=shuffle, collate_fn=collate_fn
        )

    def train_dataloader(self) -> DataLoader:
        return self._get_dataloader(self.train, shuffle=True)

The first 2 functions are not super obvious, so some comments are necessary.

The last one is pretty self-explanatory and requires no documentation (IMO).

But since we are in a phase of active development, I wouldn't enforce huge docstrings on every function. Type hints and good variable names should be enough for 95% of the code.

[sdkaraban]

are we going to add those classifications as models?

[sdkaraban]

maybe add normalization here?

[danielkorkin]

Are we just testing for 3.11? Maybe add other versions like 3.10 or eeven 3.9.

[Old-Shatterhand]

Related to this comment. I'd test all Python versions that are actively developed, such as 3.12. But if we fix versions in the requirements, I don't see why we should test multiple versions. The installed packages will have the same versions across different tested Python versions.

[danielkorkin]

Should we start specifying versions to avoid breaking changes and use dependabot to update andreview version changes?

[ilsenatorov]

yeah, sure

[danielkorkin]

This should be easily accomplished by installing requirements and then running pip freeze

[danielkorkin]

Again is 3.11 maybe too high, maybe 3.10?

[ilsenatorov]

agreed

[Old-Shatterhand]

I don't think, 3.11 is too high. I have another project with similar dependencies that is run on 3.11 and it works fine, so, I wouldn't expect any problems. But we can downgrade to 3.10 if you want.

[danielkorkin]

Yes, maybe >=3.10