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

260 remove classif #263

Merged
merged 9 commits into from
Feb 7, 2022
Merged

260 remove classif #263

merged 9 commits into from
Feb 7, 2022

Conversation

remtav
Copy link
Collaborator

@remtav remtav commented Feb 3, 2022

Also:
replace load_obj() with hydra's get_method
remove task config directory

closes #260

resuscitate train_classification.py
resuscitate inference_classification.py
add classif template config
replace load_obj() with hydra's get_method
remove task config directory
Comment on lines -1 to -2
task_name: segmentation
path_task_function: ${mode}_segmentation.main
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@CharlesAuthier let me know if there was a particular reason you wanted it like this.

Comment on lines +1 to +2
./22978945_15.tif,,./massachusetts_buildings.gpkg,,trn
./23429155_15.tif,,./massachusetts_buildings.gpkg,,tst
Copy link
Collaborator Author

@remtav remtav Feb 3, 2022

Choose a reason for hiding this comment

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

This change is temporary. Will refactor the in_case_of_path parameter. I'd rather use hydra.utils.get_original_cwd().

@@ -427,7 +424,7 @@ def try2read_csv(path_file, in_case_of_path, msg):
Path(path_file).resolve(strict=True)
except FileNotFoundError:
if in_case_of_path:
path_file = os.path.join(in_case_of_path, os.path.basename(path_file))
path_file = str(Path(in_case_of_path) / (path_file.split('./')[-1]))
try:
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I'll get back to this in_case_of_path story in a future PR. Would like to suggest a different, lighter approach with hydra.utils.get_original_cwd()

raise AttributeError(f"Object `{obj_name}` cannot be loaded from from `{obj_path}`.")
return getattr(module_obj, obj_name)


Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@CharlesAuthier same here. Let me know if you had anytning in my mind for further developments with this function. If the purpose was just to use in GDL.py to reach the right method, I think hydra.utils.get_method() does the job.

mpelchat04
mpelchat04 previously approved these changes Feb 4, 2022
@CharlesAuthier
Copy link
Collaborator

you have a conflict, after change for it I will approve

Comment on lines +562 to +563
# Save tracking TODO put option not just mlflow
if 'tracker_uri' in locals() and 'run_name' in locals():
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

No major change here, git's just having a hard time following the movement of big blocks of code.

Comment on lines +603 to +604
attribute_name=attribute_field,
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Same here. Nothing major has happened. Just deindented after removing if/else for classif/segmentation task.

@remtav remtav merged commit 6b037e1 into NRCan:develop Feb 7, 2022
@remtav remtav deleted the 260-remove-classif branch February 7, 2022 18:29
remtav added a commit to remtav/geo-deep-learning that referenced this pull request Jul 5, 2022
replace load_obj() with hydra's get_method
remove task config directory

* remove classification stuff
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.

Reimplement classification task
4 participants