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.

Sign up for GitHub

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

Jump to bottom

Major refactor manager #120

Merged
merged 34 commits into from
Feb 21, 2025
Merged

Major refactor manager #120

merged 34 commits into from
Feb 21, 2025

Conversation

mathysgrapotte
Copy link
Owner

@mathysgrapotte mathysgrapotte commented Feb 20, 2025 •
edited
Loading

major refactoring of manager system, see #100 .

Removes uneeded abstraction layers, decouples modules, makes project structure more intuitive

  • removing manager classes
  • removing loader classes
  • reorganise config loading in a data/interface/ module
  • feeding DataLoader class to TorchDataset
  • feeding TorchDataset to RaytuneLearner

closes #100
closes #101
closes #102
closes #103
renders #119 stale

note: Cli tests will fail, clis will be fixed in #116

@mathysgrapotte
docs(yaml_data): removed Yaml prefixes for added clarity.
f79641a
@mathysgrapotte
refactor(yaml_data): removed check_yaml, redundancy with pydantic fun…
e76d246 
…ctions.
@mathysgrapotte
refactor(yaml_data): made dumper cleaner.
8802c12
@mathysgrapotte
refactor(yaml_data): renamed yaml_data into yaml_data_schema for cons…
dbca1d5 
…istency.
@mathysgrapotte
docs(yaml_data): removed fixed comment and format.
fcb0c7d
@mathysgrapotte
refactor(yaml_data): splitted yaml_data_schema and data_config_parsing.
c1df9cf
@mathysgrapotte
refactor(data_config): renamed data_config_parsing to data_config_spl…
14daba8 
…itting.
@mathysgrapotte
refactor(launch_utils): renamed to model_file_interface.
47026f8
@mathysgrapotte
refactor(file_names): changed filenames to have more consistant namin…
576885e 
…g conventions.
@mathysgrapotte
refactor(config_parser): added parse_config function signature.
6a6d21a
@mathysgrapotte
make format.
31d1226
@mathysgrapotte
refactor(config_parser): added boilerplate code for instantiating enc…
fea0b87 
…oders splitters and transforms.
@mathysgrapotte
refactor(data_config_parser): data_config interface in a single module.
34049d0
@mathysgrapotte
refactor(interface): added the interface module.
90f4bb1
@mathysgrapotte
refactor(data_handlers): removed loader and manager logic.
dbae19b
@mathysgrapotte
run make format
8195319
@mathysgrapotte
refactor(handlertorch): removed handlertorched and happend it to data…
377e72e 
…_handlers.
@mathysgrapotte
refactor(data_handler): adapted TorchDataset to new paradigm.
f3f13b6
@mathysgrapotte
fix(data_handlers): removed minor bugs.
2493bfb
@mathysgrapotte
refactor(test_data_handlers): refactored tests for data_handlers.
6ffcf12
@mathysgrapotte
run make format.
02a1275
@mathysgrapotte mathysgrapotte self-assigned this Feb 20, 2025
@mathysgrapotte
Copy link
Owner Author

deals with #100 #101 #102 and #103

@mathysgrapotte mathysgrapotte added the help wanted Extra attention is needed label Feb 20, 2025
itrujnara
itrujnara reviewed Feb 20, 2025
View reviewed changes
Copy link
Collaborator

@itrujnara itrujnara left a comment

Choose a reason for hiding this comment

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

Looking pretty good, the code is much cleaner now. I've left some minor remarks, mostly on code layout.

@mathysgrapotte mathysgrapotte marked this pull request as ready for review February 20, 2025 16:23
@mathysgrapotte
Copy link
Owner Author

Ready for review, please note that CLI tests will fail, this will be addressed in #116

@mathysgrapotte
Copy link
Owner Author

this renders #119 stale @JulienRaynal

itrujnara
itrujnara approved these changes Feb 21, 2025
View reviewed changes
Copy link
Collaborator

@itrujnara itrujnara left a comment

Choose a reason for hiding this comment

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

Looks good but please remember to fix the CLIs

@mathysgrapotte mathysgrapotte merged commit a3abc79 into dev Feb 21, 2025
3 of 27 checks passed
@mathysgrapotte mathysgrapotte mentioned this pull request Feb 21, 2025
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@itrujnara itrujnara itrujnara approved these changes

@suzannejin suzannejin Awaiting requested review from suzannejin

@JulienRaynal JulienRaynal Awaiting requested review from JulienRaynal

Assignees

@mathysgrapotte mathysgrapotte

Labels
help wanted Extra attention is needed
Projects
Stimulus v1.0
Status: Done
Milestone
stimulus v1.0.0 release
Development

Successfully merging this pull request may close these issues.
2 participants
@mathysgrapotte @itrujnara