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

upgrade zoobot to latest version #56

Merged
merged 4 commits into from
Dec 12, 2024
Merged

upgrade zoobot to latest version #56

merged 4 commits into from
Dec 12, 2024

Conversation

Tooyosi
Copy link
Contributor

@Tooyosi Tooyosi commented Dec 2, 2024

Upgrade zoobot to version > 2.0.

These changes aim at upgrading zoobot to the latest version. Changes made include:

  • Upgrade the base cuda image to 12.1.1
  • Update finetune.FinetuneableZoobotTree to match with expected params
  • Change transform to test_transform from galaxy_datasets
  • Import euclid_ortho_schema for new euclid workflow usecase

@yuenmichelle1 yuenmichelle1 linked an issue Dec 2, 2024 that may be closed by this pull request
@yuenmichelle1
Copy link
Contributor

yuenmichelle1 commented Dec 2, 2024

Hey @Tooyosi
I probably am not the best person to review since my knowledge of Bajor flow is super limited. Yay to replacing that hard coded euclid schema 🥇 🎉

  • I think it would be helpful to add within your PR comments, the changes that were made:
    Eg.
    Update predict_dataset to utilize new test_transform, replacing transform (Introduced in galaxy_datasets v0.0.17, galaxy_datasets used in zoobot)
    ^ It doesnt need to be this detailed, but I think super helpful for future devs to know why certain params are changed, etc.

  • Probably also helpful to list what setup is already done and what setup still needs to be done to get this working in Azure Batch.
    Eg.

  • TODO: Once merged, Push image to ContainerRegistry with pytorch 2.0 and create new batch pool. etc.

^ Maybe overkill but I like to see the process, plus helpful for our future selves if we have to do this kind of update again. 😄

That's at least my two cents. I'll remove myself as reviewer since my review (other than the above comments) is not going to be as fruitful as reviews from Mike et al.

@yuenmichelle1 yuenmichelle1 removed their request for review December 2, 2024 23:42
Copy link
Member

@lcjohnso lcjohnso left a comment

Choose a reason for hiding this comment

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

LGTM!

I'll note that pinning to python3.9 and overriding the system python3 version is OK (best to mimic Mike's recommendations for Zoobot), but using the image's default (3.10?) would be better -- prevent issues with the image AND buys us another year before Python version EOL (3.9 is EOL in late 2025).

Excited to see this up and running!

@Tooyosi Tooyosi merged commit d938268 into main Dec 12, 2024
1 check passed
@mwalmsley
Copy link
Collaborator

Zoobot should work on 3.10 and above, I just haven't checked it myself. I'll make a note to do that next update.

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.

Remove hard coded schema
4 participants