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

Restructure from scripts #8

Closed
jatkinson1000 opened this issue Feb 16, 2023 · 4 comments
Closed

Restructure from scripts #8

jatkinson1000 opened this issue Feb 16, 2023 · 4 comments
Labels
code review Resulting from Feb 2023 code review question Further information is requested

Comments

@jatkinson1000
Copy link
Member

jatkinson1000 commented Feb 16, 2023

The code is currently run using scripts as main entry points (cmip26.py and trainScript.py).

This is not ideal for distributing as they are long scripts that a user needs to run and change details ('magic numbers') in.
These will also not work in the proposed src/ layout.

Scripts should be broken up in to functions with simple entry/call in main package, and grouped with other relvant code.

However, there is a larger discussion that needs to be held to decide how to break up the code.

  • Separate libraries within this repo for data acquisition/processing and model?

  • Separate repositories for data acquisition/processing and model?

    • This starts to feel verbose, but is what is required for Huggingface?
  • cmip26.py probably needs to be grouped with data acquisition and processing src/gz_ocean_momentum/data/

  • trainScript.py probably needs to be grouped with src/gz_ocean_momentum/train/

Related:

@jatkinson1000 jatkinson1000 added question Further information is requested code review Resulting from Feb 2023 code review labels Mar 1, 2023
@raehik
Copy link
Contributor

raehik commented May 16, 2023

This is the next large refactoring that we should look at. Mostly a question of how we configure runs, and optionally MLflow. (The training stage in particular I think depends on MLflow to locate data to process. The data processing stage cmip26.py I was able to run without MLflow.)

@raehik
Copy link
Contributor

raehik commented Sep 20, 2023

#85 addresses this for the data step.

@MarionBWeinzierl
Copy link
Collaborator

#95 does the training step refactor

@raehik
Copy link
Contributor

raehik commented Nov 8, 2023

We're closing this issue as partially done:

@raehik raehik closed this as completed Nov 8, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
code review Resulting from Feb 2023 code review question Further information is requested
Projects
None yet
Development

No branches or pull requests

4 participants