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

fix_box_loss_error #313

Merged
merged 2 commits into from
Sep 17, 2024
Merged

Conversation

venkatram-dev
Copy link
Contributor

@venkatram-dev venkatram-dev commented Aug 20, 2024

Description

Please include a summary of the change and which issue is fixed or implemented. Please also include relevant motivation and context (e.g. links, docs, tickets etc.).

List any dependencies that are required for this change.

Related to this issue #311.

I debugged by outputting the contents of epoch here https://github.com/roboflow/roboflow-python/blob/main/roboflow/core/version.py#L414

Somehow for the last epoch , we do not find the "loss" values . (It shows up in UI though, but somehow not in this dict).

Here is the output from my debugging. Please refer the last epoch 167

epoch in for loop {'recall': 0.5, 'mAP': 0.0072, 'obj_loss': 2.61305, 'ts': 1724181787.4027717, 'box_loss': 3.15483, 'mAP_50_95': 0.00203, 'class_loss': 3.7698, 'precision': 0.00667, 'epoch': 166}

epoch in for loop {'precision': 0.00667, 'mAP': 0.0072, 'box_loss': 3.15483, 'ts': 1724181788.498687, 'obj_loss': 2.61305, 'class_loss': 3.7698, 'epoch': 167, 'recall': 0.5, 'mAP_50_95': 0.00203}

epoch in for loop {'mAP': 0.28003333333333325, 'ts': 1724181792.7439988, 'precision': 0.9130414197981767, 'mAP_50_95': 0.10775882882882881, 'epoch': 167, 'recall': 0.25}

So, I am proposing defensive coding and adding the "loss" only if those keys exist in the dictionary.

https://app.roboflow.com/test-4h9f7/test2-forest-fire-detection/6

Type of change

Please delete options that are not relevant.

How has this change been tested, please provide a testcase or example of how you tested the change?

From the UI, Created a new project, new version with sample dataset, created version.

I then modified the code (as in the PR) in my virtual environment and tested it.

rf = roboflow.Roboflow(api_key=YOUR_API_KEY)

workspace = rf.workspace()

project = workspace.project('test2-forest-fire-detection')

version = project.version(6)

print ('version',version)

version.train()

PS/ Side Note : There is one more bug in the code where we get error https://github.com/roboflow/roboflow-python/blob/main/roboflow/core/version.py#L450 . But we can probably take it as a separate PR.
Opened a new issue for tracking that.

assert self.model
           ^^^^^^^^^^
AssertionError

Any specific deployment considerations

For example, documentation changes, usability, usage/costs, secrets, etc.

Docs

  • Docs updated? What were the changes:

@venkatram-dev
Copy link
Contributor Author

venkatram-dev commented Aug 27, 2024

@grzegorz-roboflow , @ https://github.com/HahaBill. Please review.

@HahaBill
Copy link

@grzegorz-roboflow , @ https://github.com/HahaBill. Please review.

Hi, I am unfortunately not from the Roboflow team. Therefore, I cannot review or allow your changes to be merged.

@venkatram-dev
Copy link
Contributor Author

venkatram-dev commented Sep 3, 2024

@LinasKo , can you please review this .
My code change here will fix the error, but I am not sure how to find why the UI works differently.
Please let me know where can i find the code for it.

Somehow for the last epoch , we do not find the "loss" values . (It shows up in UI though, but somehow not in this dict).

@grzegorz-roboflow
Copy link
Contributor

Hi @venkatram-dev, thank you for submitting this contribution, I merged the change!

@grzegorz-roboflow grzegorz-roboflow merged commit db89776 into roboflow:main Sep 17, 2024
6 checks passed
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.

3 participants