Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Ensure pixel SOM training reruns if new markers specified #903

Merged
merged 4 commits into from
Feb 10, 2023

Conversation

alex-l-kong
Copy link
Contributor

What is the purpose of this PR?

Closes #887. It's currently not possible to re-run 2_Pixie_Cluster_Pixels in case the user needs to specify a new subset of markers on the same cohort. This is a common occurrence and should be addressed.

How did you implement your changes

In PixelSOMCluster.train_som, add a check to verify the existing weights columns (if applicable) and the provided set of columns match. If they do not, this means a new set of columns was specified and PixieSOMCluster.train_som should re-run.

@alex-l-kong alex-l-kong requested a review from cliu72 February 7, 2023 22:28
Copy link
Contributor

@cliu72 cliu72 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it would be a good idea to add this functionality to train_som in the cell version as well. I think it could come up in the general cell clustering case (for example, in Lorenz's case if he wants to re-run the training and leave out some markers)

@alex-l-kong
Copy link
Contributor Author

I think it would be a good idea to add this functionality to train_som in the cell version as well. I think it could come up in the general cell clustering case (for example, in Lorenz's case if he wants to re-run the training and leave out some markers)

Yeah that's a good point, I had mentioned it in the discussion on the generalized cell clustering PR but definitely better to do it here.

@alex-l-kong alex-l-kong requested a review from cliu72 February 9, 2023 22:29
Copy link
Contributor

@cliu72 cliu72 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good!

@alex-l-kong alex-l-kong merged commit 44af617 into main Feb 10, 2023
@alex-l-kong alex-l-kong deleted the retrain_pixel_som branch February 10, 2023 20:24
@srivarra srivarra added the bug Something isn't working label Feb 16, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2_pixie cluster pixel does not retrain SOM when new channel added
3 participants