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

Hotfix: Reorganize conditional_train code #81

Closed
5 tasks done
chrstnkgn opened this issue Jan 17, 2023 · 1 comment · Fixed by #89
Closed
5 tasks done

Hotfix: Reorganize conditional_train code #81

chrstnkgn opened this issue Jan 17, 2023 · 1 comment · Fixed by #89
Assignees
Labels
Bug Something isn't working Hydra About Hydra (experiment management) Trainer About benchmark trainer file

Comments

@chrstnkgn
Copy link
Collaborator

chrstnkgn commented Jan 17, 2023

What

Reorganize conditional train code, and simplify its config

Why

  • We are aware that we should somehow modify the code for conditional training, but it has been delayed as it was not a priority.
  • However, as we are preparing for our first release, neater (and well-working) code is needed.
  • This issue will cover following parts:
    1. (conditional_train.py) Merge train and trainval function - this format is not necessary anymore as we are not using ray tune for conditional learning, and this will resolve some error that was caused by its complicated format.
    2. (conditiona_train.yaml) Modify its format - As this is not a major option that the users will always take into account, it would be better to place this config somewhere in the lower level. Trimming some unnecessary configs is also needed.

How

  • conditional_train.py
    • merge train and trainval
    • remove unnecessary codes
  • conditional_train.yaml
    • Modify format
@chrstnkgn chrstnkgn self-assigned this Jan 17, 2023
@chrstnkgn chrstnkgn added Bug Something isn't working Hydra About Hydra (experiment management) Trainer About benchmark trainer file labels Jan 17, 2023
@chrstnkgn chrstnkgn added this to the Modality: X-ray milestone Jan 17, 2023
@seoulsky-field
Copy link
Owner

Great issue! I was thinking when we revise this and how we do that. And I agree this is the best time to do now. Thanks for your efforts. If you have any supports to do this issue, I'll help right away.

@chrstnkgn chrstnkgn linked a pull request Feb 1, 2023 that will close this issue
chrstnkgn added a commit that referenced this issue Feb 2, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Something isn't working Hydra About Hydra (experiment management) Trainer About benchmark trainer file
Projects
Status: Done
Development

Successfully merging a pull request may close this issue.

2 participants