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

DOCS-3423: Update training script to parse labels based on feedback from etai/tahiya #3941

Open
wants to merge 15 commits into
base: main
Choose a base branch
from

Conversation

sguequierre
Copy link
Collaborator

@sguequierre sguequierre commented Jan 29, 2025

  • parsing labels and adding validation for num_epochs (used later if they decide to implement a fit function)
  • removing google cloud imports bc already gets imported
    Manual testing notes:
  • did test and successfully submitted training job with the labels which succeeded with cli (not with UI bc you have to put labels in the arg, put notes for that on docs and made alternate template with the corrections I found but without parsing labels)

@viambot viambot added the safe to build This pull request is marked safe to build from a trusted zone label Jan 29, 2025
Copy link

netlify bot commented Jan 29, 2025

Deploy Preview for viam-docs ready!

Name Link
🔨 Latest commit 4c57132
🔍 Latest deploy log https://app.netlify.com/sites/viam-docs/deploys/67a4d084f56a8200070d9d78
😎 Deploy Preview https://deploy-preview-3941--viam-docs.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.
Lighthouse
Lighthouse
1 paths audited
Performance: 21 (🔴 down 11 from production)
Accessibility: 98 (no change from production)
Best Practices: 92 (🔴 down 8 from production)
SEO: 92 (no change from production)
PWA: 70 (no change from production)
View the detailed breakdown and full score reports

To edit notification comments on pull requests, go to your Netlify site configuration.

docs/data-ai/ai/train.md Outdated Show resolved Hide resolved


# This is used for parsing the dataset file (produced and stored in Viam),
# parse it to get the label annotations
# Used for training classifiction models


Copy link
Collaborator Author

Choose a reason for hiding this comment

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

flake8 parser was complaining about this


# Save labels.txt file
save_labels(LABELS + [unknown_label], MODEL_DIR)
# Convert the model to tflite
save_model(
model, MODEL_DIR, "classification_model", IMG_SIZE + (3,)
model, MODEL_DIR, "classification_model"
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

correction, was never being used

Copy link
Member

@vpandiarajan20 vpandiarajan20 left a comment

Choose a reason for hiding this comment

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

LGTM! Make sure to try out the script on Viam and add some details about manual testing in the PR comments, then it'll be good to merge


# This parses the required args for the training script.
# The model_dir variable will contain the output directory where
# the ML model that this script creates should be stored.
# The data_json variable will contain the metadata for the dataset
# that you should use to train the model.


Copy link
Collaborator Author

Choose a reason for hiding this comment

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

know this is awk but python markdown parser was complaining about this specifically

docs/data-ai/ai/train.md Outdated Show resolved Hide resolved
docs/data-ai/ai/train.md Outdated Show resolved Hide resolved
docs/data-ai/ai/train.md Outdated Show resolved Hide resolved
@@ -90,15 +88,17 @@ If you haven't already, create a folder called <file>model</file> and create an

<p><strong>4. Add <code>training.py</code> code</strong></p>

<p>Copy this template into <file>training.py</file>:</p>
<p>Copy one of the following templates into <file>training.py</file>, depending on whether or not you wish to parse labels:</p>
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can you rephrase this to have the condition based on which the reader should decide be clearer? Why would a user need to use the labels flag or not?

Copy link
Collaborator

@npentrel npentrel left a comment

Choose a reason for hiding this comment

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

see comment about conditional

docs/data-ai/ai/train.md Outdated Show resolved Hide resolved
docs/data-ai/ai/train.md Outdated Show resolved Hide resolved
docs/data-ai/ai/train.md Outdated Show resolved Hide resolved
@sguequierre sguequierre requested a review from npentrel February 5, 2025 16:16
docs/data-ai/ai/train.md Outdated Show resolved Hide resolved
docs/data-ai/ai/train.md Outdated Show resolved Hide resolved
docs/data-ai/ai/train.md Outdated Show resolved Hide resolved
@sguequierre sguequierre requested a review from npentrel February 6, 2025 15:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
safe to build This pull request is marked safe to build from a trusted zone
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants