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

Refacto to use Lightning CLI #124

Merged
merged 13 commits into from
Jan 9, 2025
Merged

Refacto to use Lightning CLI #124

merged 13 commits into from
Jan 9, 2025

Conversation

LBerth
Copy link
Contributor

@LBerth LBerth commented Jan 8, 2025

  • rm TorchDataLoaderSettings class
  • rm ArLightningHyperParams and define all these variables as attributes of the ARLightningModule
  • Add bin/main.py as alternative to bin/train.py, using lightning CLI to launch fit : python bin/main.py fit --config bin/config_test_cli.yaml or python bin/main.py test --config bin/config_test_cli.yaml.
  • Add yaml file config_test_cli as basic titan config for CLI
  • rm old file ideas/test_argpase.py (old alternative to CLI)
  • Nota bene: bin/train.py still works, but inference.py, test.py and gif comparaison will probably not work but will be replaced in future PR.

@LBerth LBerth marked this pull request as ready for review January 8, 2025 16:10
clemoule
clemoule approved these changes Jan 8, 2025
@clemoule clemoule merged commit 16e4ac3 into meteofrance:main Jan 9, 2025
1 check passed
Copy link
Contributor

@flyIchtus flyIchtus left a comment

Choose a reason for hiding this comment

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

Very cool, nothing that bothers me, the code seems even more readable (I even seem to understand what cli is about 😅 )

bin/gif_comparison.py Show resolved Hide resolved
bin/gif_comparison.py Show resolved Hide resolved
bin/main.py Show resolved Hide resolved
py4cast/lightning.py Show resolved Hide resolved
@flyIchtus
Copy link
Contributor

oups I am too late

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