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

XGboost Changes for ocean-prefilter #14

Merged
merged 14 commits into from
Jul 23, 2024
Merged

Conversation

Emerald-Z
Copy link
Contributor

Main contributions:

  • rewriting of the classification logic to use xgboost instead of histogram comparison
  • update of the README to be consistent with changes
  • adding test files to test the new logic
  • making the camera_name an optional parameter

@Emerald-Z Emerald-Z requested a review from bhaney July 22, 2024 15:54
Copy link
Member

@bhaney bhaney left a comment

Choose a reason for hiding this comment

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

Nice first pass on adding xgboost to this package, I think it'll be the way going forward! I have comments on writing tests, and I would also recomment running the linter too so that it catches any style issues in the source code

README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
oceanprefilter/oceanprefilter.go Outdated Show resolved Hide resolved
oceanprefilter/oceanprefilter.go Outdated Show resolved Hide resolved
oceanprefilter/prefilter_test.go Outdated Show resolved Hide resolved
oceanprefilter/test.go Outdated Show resolved Hide resolved
oceanprefilter/utils.go Outdated Show resolved Hide resolved
oceanprefilter/utils.go Outdated Show resolved Hide resolved
oceanprefilter/xgboost.go Outdated Show resolved Hide resolved
@Emerald-Z Emerald-Z requested a review from bhaney July 22, 2024 19:19
Copy link
Member

@bhaney bhaney left a comment

Choose a reason for hiding this comment

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

Thanks for the edits! Here are a few more small corrections to do

oceanprefilter/oceanprefilter.go Show resolved Hide resolved
oceanprefilter/oceanprefilter.go Show resolved Hide resolved
example/main.go Show resolved Hide resolved
oceanprefilter/prefilter_test.go Outdated Show resolved Hide resolved
oceanprefilter/prefilter_test.go Outdated Show resolved Hide resolved
oceanprefilter/prefilter_test.go Outdated Show resolved Hide resolved
oceanprefilter/utils.go Outdated Show resolved Hide resolved
oceanprefilter/utils.go Outdated Show resolved Hide resolved
@Emerald-Z Emerald-Z requested a review from bhaney July 23, 2024 17:03
Copy link
Member

@bhaney bhaney left a comment

Choose a reason for hiding this comment

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

LGTM!

@Emerald-Z Emerald-Z merged commit 1c90b5c into viamrobotics:main Jul 23, 2024
2 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.

2 participants