Skip to content

Update scprint#71

Merged
mumichae merged 23 commits into
openproblems-bio:mainfrom
jkobject:main
Sep 29, 2025
Merged

Update scprint#71
mumichae merged 23 commits into
openproblems-bio:mainfrom
jkobject:main

Conversation

@jkobject

Copy link
Copy Markdown
Contributor

update scprint to v2.3.5

@rcannood rcannood left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM! Thanks for improving the scprint component!

@rcannood

Copy link
Copy Markdown
Member

@mumichae Can this be merged?

@rcannood rcannood changed the title update scprint Update scprint Aug 20, 2025
@mumichae mumichae requested a review from lazappi August 29, 2025 09:22
@mumichae

mumichae commented Aug 29, 2025

Copy link
Copy Markdown
Collaborator

I noticed that the preprocessing is inconsistent compared to all the other integration method, since you're not taking the HVGs that have been provided and likely do your own preprocessing and feature selection via scdataloader.Prreprocessor. I opened a new issue for that: #81

Comment thread src/methods/scprint/config.vsh.yaml
Comment thread src/methods/scprint/script.py Outdated
Comment thread src/methods/scprint/script.py Outdated
@lazappi

lazappi commented Aug 29, 2025

Copy link
Copy Markdown
Member

Made some minor comments

I noticed that the preprocessing is inconsistent compared to all the other integration method, since you're not taking the HVGs that have been provided and likely do your own preprocessing and feature selection via scdataloader.Prreprocessor. I opened a new issue for that: #81

I think the preprocessor is just checking that enough genes are present, not doing feature selection, and that removing features would be bad for the model. But I suggest you discuss with @jkobject on #81.

Co-authored-by: Luke Zappia <lazappi@users.noreply.github.com>
Comment thread src/methods/scprint/script.py Outdated
@mumichae

Copy link
Copy Markdown
Collaborator

I think the preprocessor is just checking that enough genes are present, not doing feature selection, and that removing features would be bad for the model. But I suggest you discuss with @jkobject on #81.

Thanks Luke. If that's the case, then it's all good on my side 👍

@lazappi Can this be merged?

@lazappi

lazappi commented Sep 26, 2025

Copy link
Copy Markdown
Member

I think so if you're happy with it

@jkobject

Copy link
Copy Markdown
Contributor Author

@lazappi btw I have a good docker image for scGPT with flashattention for the grn_inference benchmark, do you want it for the other ones?

@lazappi

lazappi commented Sep 29, 2025

Copy link
Copy Markdown
Member

@jkobject If you want to open some PRs that would be helpful. I think the scGPT components need an update anyway.

@mumichae mumichae merged commit 1c34b59 into openproblems-bio:main Sep 29, 2025
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants