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

92 training pipeline #187

Merged

Conversation

jogong2718
Copy link
Contributor

Finished the training script and awaiting review.

Additional details:

  • The transform_data function can be changed to remove the line that removes null values. This is because testing the interim datasets there are null values in the bgl column but I expect the processed data to not have those
  • As we are only predicting the largest meals, the ground truth labels are only 0 and 1 (for the "ANNOUNCE_MEAL"). This can be changed depending on future prediction needs
  • _init_.py files were added to some directories so the file paths would work

Note: When working with importing the ClusterSegmenter, the documentation for the module that the model is implemnted from the sktime website differed from the actual module name. The website states sktime.annotation.cluster when the actual module is sktime.annotation.clust.

jogong2718 and others added 30 commits October 15, 2024 16:54
…rint statements with logger for improved logging
… logging for training process, and improve error handling during model fitting
@jogong2718
Copy link
Contributor Author

Sorry for the delay @RobotPsychologist. I just finished configuring the unit tests so I think this issue should be done.

@RobotPsychologist
Copy link
Owner

No worries @jogong2718 thanks for doing this, seems like its still failing one test though. @Tony911029 do you have any thoughts on this?

@Tony911029
Copy link
Collaborator

Tony911029 commented Nov 28, 2024

I will take a look now. @jogong2718 Can you invite me to your repo? I can take a look there. A bit hard for me fix this without seeing the actual branch locally

@jogong2718
Copy link
Contributor Author

@Tony911029 Just added you

@NathanL15
Copy link
Contributor

Config in linux server error is probably due to the two "meal_identification" folders. Check if it applies to you @Tony911029 and try both folder directories to see if the error persists. Also, you might need to add another meal identification to the config directory as well once you get config recognized.

@Tony911029
Copy link
Collaborator

@NathanL15 I don't think it is nested meal_identification's issues because it ran before. I will do some comparison with existing main

@Tony911029
Copy link
Collaborator

@NathanL15 Fixed. I think you added some init so the package was imported from the wrong place

@Tony911029
Copy link
Collaborator

@paulpark6 I think the test you just pushed failed

@paulpark6
Copy link
Contributor

Working on the issue, my bad.

Copy link
Owner

@RobotPsychologist RobotPsychologist left a comment

Choose a reason for hiding this comment

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

Why are all these venv files in the PR? Why is there 876 files? lol!

Copy link
Contributor Author

@jogong2718 jogong2718 left a comment

Choose a reason for hiding this comment

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

I think the "update repo" commit may have caused the 897 file changes. I'm going to see if I can revert these changes.

Copy link
Collaborator

Choose a reason for hiding this comment

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

This init will cause some problems. Is there a reason why this is included?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'll remove it. Im not sure why it was added I think it was to try and fix the module import issue but that didnt help.

@jogong2718
Copy link
Contributor Author

Ok @RobotPsychologist @Tony911029 . Things should be fixed.

Copy link
Owner

@RobotPsychologist RobotPsychologist left a comment

Choose a reason for hiding this comment

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

I think this should be good I don't see any immediate issues!

@RobotPsychologist RobotPsychologist merged commit c5e6fc2 into RobotPsychologist:main Nov 29, 2024
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

5 participants