-
Notifications
You must be signed in to change notification settings - Fork 41
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
Feature/classifier metrics #33
Conversation
Trying to fix travis build https://travis-ci.org/tue-robotics/image_recognition/builds/306472861
image_recognition_tools/setup.py
Outdated
from catkin_pkg.python_setup import generate_distutils_setup | ||
|
||
d = generate_distutils_setup( | ||
packages=['image_recognition_tools'], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not required since it does not contain any code in the specified module
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also the 'src' dir can be fully removed
if args.output: | ||
plt.savefig(args.output) | ||
else: | ||
plt.show() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No newline
I thought I needed this to get the build working
@reinzor reviews processed |
|
||
find_package(catkin REQUIRED COMPONENTS) | ||
|
||
catkin_python_setup() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can be removed because it's not a python module (only a script)
image_recognition_tools/setup.py
Outdated
@@ -0,0 +1,6 @@ | |||
from distutils.core import setup |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This file can also be removed
print ">> Labels: {}".format(csm.labels) | ||
print ">> Accuracy: {acc:.3f}".format(acc=accuracy) | ||
|
||
plot_false_positive_true_positive_rates(csm.labels, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Creating a figure on the side of the user allows to arrange the plots and subplots/figures by the user.
This is also the place to put a title etc.
I agree, but this code is way more clean than it was. The user can still
set titles etc but indeed the subplot features are gone now. Would you
suggest to move the figure() call to the user side?
…-Rein
On Sat, Dec 9, 2017 at 9:57 PM, Loy ***@***.***> wrote:
***@***.**** commented on this pull request.
------------------------------
In image_recognition_analysis/scripts/classifier_metrics
<#33 (comment)>
:
> +parser = argparse.ArgumentParser(description='Calculate various metrics of a classifier that assigns a score per class')
+parser.add_argument("input", type=str, help='Input csv file. First column is ground truth label, other columns are the '
+ 'per-class scores, as output by `evaluate_classifier`',
+ default="result.csv")
+args = parser.parse_args()
+
+# Construct the classification score matrix from the input csv in order to perform all calculations
+csm = ClassificationScoreMatrix.from_file(args.input)
+
+accuracy = accuracy_score(csm.classifications_ground_truth, csm.classifications_predicted_label)
+
+print ">> # Classifications: {}".format(len(csm.classifications_ground_truth))
+print ">> Labels: {}".format(csm.labels)
+print ">> Accuracy: {acc:.3f}".format(acc=accuracy)
+
+plot_false_positive_true_positive_rates(csm.labels,
Creating a figure on the side of the user allows to arrange the plots and
subplots/figures by the user.
This is also the place to put a title etc.
—
You are receiving this because you modified the open/close state.
Reply to this email directly, view it on GitHub
<#33 (review)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AD-4l4hzDGXKWdeH9z9v_tSfGt4Nd8aoks5s-vRSgaJpZM4QmcWu>
.
|
Yes I would. Low prio though |
This PR re-adds the classifier metrics that were removed from #30 to make the PR cleaner.