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

Missing segment.py and classify.py for model finetune #299

Closed
dokterbob opened this issue Jul 10, 2024 · 3 comments
Closed

Missing segment.py and classify.py for model finetune #299

dokterbob opened this issue Jul 10, 2024 · 3 comments

Comments

@dokterbob
Copy link

dokterbob commented Jul 10, 2024

As referred to from README's:

Maybe the README's are wrong, and there's another way to train?

I don't know, never did PyTorch Lightning -- but I guess we should have a file with L.Train() somewhere?

P.S. ❤️ ❤️ ❤️ your work. We want to build upon it with https://treescape.app

@dokterbob
Copy link
Author

Ref: #275

@yellowcap
Copy link
Member

@dokterbob these files actually sit at the root of the model repo. This is to be able to run them straight off the model repo folder. But your question makes me think that we should reconsider this and maybe move them in to their respective finetune folders.

@srmsoumya let's think about this when we integrate these examples into the docs better.

https://github.com/Clay-foundation/model/blob/main/segment.py
https://github.com/Clay-foundation/model/blob/main/classify.py
https://github.com/Clay-foundation/model/blob/main/regression.py

Thanks for the kind words @dokterbob ! Great to see enthusiasm around our model!

@dokterbob
Copy link
Author

dokterbob commented Jul 13, 2024

@yellowcap Thanks for getting back. How silly, I'd swore I'd looked at that. I see they were added some weeks ago, perhaps I was looking before that? confusedface

Reviewing the contents though, they're just wrappers with hard dependencies on their respective fine-tuning module. From that perspective, yes, they belong in those sections.

If and when I start giving this a test drive I can fire a PR.

Then again, all they do is refer to a data module and the model. Perhaps we don't need different entry points for that, rather just a --datamodule and --model argument on the trainer.py entry point. You'd specify the import path and the script does a dynamic import, e.g. finetune.segment.chesapeake_datamodule.ChesapeakeDataModule.

Actually, I believe that the Pytorch Lightning CLI already does that. Less code means less maintenance on code and docs for you! ;)

That would make it fully generalisable to actual use cases -- where people will always want to train on their own data (I do...!).

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

No branches or pull requests

2 participants