Partial Grading #152

ankitjavalkar · 2016-11-07T06:33:42Z

Add partial grading to evaluation

prathamesh920

I have few comments.
Also, we should add test cases for testing partial grade marks returned from the evaluator.

prathamesh920 · 2016-11-07T08:21:10Z

yaksh/bash_code_evaluator.py

@@ -27,7 +28,7 @@ def teardown(self):
            delete_files(self.files)
        super(BashCodeEvaluator, self).teardown()

-    def check_code(self, user_answer, file_paths, test_case):
+    def check_code(self, user_answer, file_paths, partial_grading, test_case, marks):


check_code now returns 3 values, so it will be better if the Returns description is modified in the method docstring.

I will add further test cases & validations in a subsequent PR, I was hoping this could be merged so that further Evaluator based PRs can be designed accordingly

The docstring can be updated now though.

prathamesh920 · 2016-11-07T08:49:25Z

yaksh/models.py

@@ -1104,9 +1115,11 @@ class TestCase(models.Model):

 class StandardTestCase(TestCase):
    test_case = models.TextField(blank=True)
+    marks = models.FloatField(default=0.0)


So each test case will have marks!
Instead I think a single constant fraction value for each testcase is a better option.
Else we can sum the total testcases and find the ratio of right to total.

Agree with @prathamesh920 !

prathamesh920 · 2016-11-07T08:54:55Z

yaksh/models.py

        user_answer.correct = correct
        user_answer.error = result.get('error')
+        if correct:


If we have a fraction(value 0 to 1) returned from the code sever, then we do not need this if block, we can simply write
user_answer.marks = question.points * result['marks']
if wrong then marks is 0
if right then marks is 1
else fraction of marks.
So no need to check correct or wrong for partial grading when adding marks.

If condition will have to stay.
In case when there is no partial grading for a question, in that case the evaluator always returns 0.0
So if a question is answered correctly but it is not graded partially then it will end up being marked 0.0 marks (in case success=True is not checked)

I agree with @prathamesh920 that each test case having marks is non optimal. I think I have a nicer way to specify this. Basically tread the "marks" you have above as a weight for each test, then the total mark is sum(obtained weight)/total(weight)*marks. By default each test case has a weight of 1, this way is nicer I think and much easier to implement. A teacher may even change the total marks without having to worry about the individual weights.

prabhuramachandran

Apart from the comments made, I am OK with this being a two stage PR, however, it will be good to resolve the current issues.

prabhuramachandran · 2016-11-07T18:46:12Z

yaksh/bash_code_evaluator.py

@@ -27,7 +28,7 @@ def teardown(self):
            delete_files(self.files)
        super(BashCodeEvaluator, self).teardown()

-    def check_code(self, user_answer, file_paths, test_case):
+    def check_code(self, user_answer, file_paths, partial_grading, test_case, marks):


The docstring can be updated now though.

prabhuramachandran · 2016-11-07T18:51:57Z

yaksh/models.py

        user_answer.correct = correct
        user_answer.error = result.get('error')
+        if correct:


I agree with @prathamesh920 that each test case having marks is non optimal. I think I have a nicer way to specify this. Basically tread the "marks" you have above as a weight for each test, then the total mark is sum(obtained weight)/total(weight)*marks. By default each test case has a weight of 1, this way is nicer I think and much easier to implement. A teacher may even change the total marks without having to worry about the individual weights.

prabhuramachandran · 2016-11-07T18:52:28Z

yaksh/models.py

@@ -1104,9 +1115,11 @@ class TestCase(models.Model):

 class StandardTestCase(TestCase):
    test_case = models.TextField(blank=True)
+    marks = models.FloatField(default=0.0)


Agree with @prathamesh920 !

…ased partial grading

ankitjavalkar · 2016-11-11T10:29:49Z

Docstrings added.
Test cases now have point based weightage

prathamesh920

There are few comments, kindly resolve them and test them.
I also feel that there should be testcases for partial grading, though can be added in stage 2 while optimizing.

prathamesh920 · 2016-11-13T02:59:36Z

yaksh/bash_code_evaluator.py

@@ -27,7 +28,7 @@ def teardown(self):
            delete_files(self.files)
        super(BashCodeEvaluator, self).teardown()

-    def check_code(self, user_answer, file_paths, test_case):
+    def check_code(self, user_answer, file_paths, partial_grading, test_case, weightage):


docstring not modified. Can be done in next stage.

Instead of weightage perhaps use weight.

prathamesh920 · 2016-11-13T03:05:52Z

yaksh/views.py

@@ -517,11 +517,16 @@ def check(request, q_id, attempt_num=None, questionpaper_id=None):
                        if question.type == 'code' else None
        correct, result = paper.validate_answer(user_answer, question, json_data)
        if correct:
+            new_answer.marks = (question.points * result['weightage'] / 


in code_evaluator line number 92 is following
result = {'success': success, 'error': error, 'marks': marks}
so here result['weightage'] will throw an error!

I had already fixed this in a minor commit, It did not get pushed. I apologise. This has been fixed

prathamesh920 · 2016-11-13T03:22:18Z

yaksh/views.py

            new_answer.error = result.get('error')
        else:
            new_answer.error = result.get('error')
+            new_answer.marks = (question.points * result['weightage'] /
+                question.get_maximum_test_case_weightage()) \
+                if question.partial_grading and question.type == 'code' else 0
        new_answer.save()
        paper.update_marks('inprogress')
        paper.set_end_time(timezone.now())


Below we see that there is check if not result.get('success'):
in else we add the current question to completed questions and show the next question.
So in case my all testcases fail except for the last one then the question will be added to the completed questions!!!

This has been fixed now

prathamesh920 · 2016-11-13T03:37:08Z

yaksh/code_evaluator.py

+                    weightage += test_case_weightage
+                    error = err
+                else:
+                    error += err + "\n"


So for all testcases we are appending the err.
In case, the first testcase passes and the next testcase fails then
error message will be Correct Answer + Assertion Error.

This can be covered in your next PR related to error message formatting,

So maybe error should be a list?

prabhuramachandran · 2016-11-13T08:01:07Z

yaksh/bash_code_evaluator.py

            else:
                err = ("Error:expected"
                    " {0}, got {1}").format(inst_stdout+inst_stderr,
                        stdnt_stdout+stdnt_stderr
                    )
-                return False, err
+                return False, err, test_case_weightage


Why is this returning the weightage?

I have initialised the test_case_weightage as 0.0 earlier. Hence the variable is returned as is.

However I agree that this may be a bit unclear, hence fixing this to return a constant 0.0

prabhuramachandran · 2016-11-13T08:02:27Z

yaksh/xmlrpc_clients.py

-            result = json.dumps({'success': False, 'error': 'Unable to connect to any code servers!'})
+            result = json.dumps({'success': False,
+                'weightage': 0.0,
+                'error': 'Unable to connect to any code servers!'})


Please describe what the return values are in the docstring very clearly. This looks like the sum of the weights that the user code passed, correct?

Yes, that is correct, Fixed the docstrings accordingly

ankitjavalkar · 2016-11-16T15:06:35Z

I have changed the Error display in the same PR (because both were interdependent)

Please check the Output Display Interface:

ankitjavalkar · 2016-11-17T05:19:26Z

Added a Test case to verify the situation pointed out by @prathamesh920

prabhuramachandran · 2016-11-17T06:56:23Z

yaksh/bash_code_evaluator.py

        test/have dissimilar output, when compared to the instructor script.

-        Returns (False, error_msg): If mandatory arguments are not files or if
+        Returns (False, error_msg, 0.0): If mandatory arguments are not files or if


In this case, should this just raise a custom exception?

prabhuramachandran · 2016-11-17T06:58:32Z

yaksh/bash_code_evaluator.py

        if file_paths:
            self.files = copy_files(file_paths)
        if not isfile(clean_ref_code_path):
            msg = "No file at %s or Incorrect path" % clean_ref_code_path
-            return False, msg
+            return False, msg, 0.0


All these should perhaps raise an exception, no?

prabhuramachandran · 2016-11-17T06:59:45Z

yaksh/code_evaluator.py

+                if test_case_success:
+                    weight += test_case_weight
+
+                error += err + "\n"


This is dirty, can't we collect a list of errors?

prabhuramachandran · 2016-11-17T07:04:05Z

yaksh/python_assertion_evaluator.py

@@ -47,11 +68,13 @@ def check_code(self, user_answer, file_paths, test_case):
            info = traceback.extract_tb(tb)
            fname, lineno, func, text = info[-1]
            text = str(test_case).splitlines()[lineno-1]
-            err = "{0} {1} in: {2}".format(type.__name__, str(value), text)
+            err = ("-----\nExpected Test Case:\n{0}\n"


Looks ugly, can't it be some HTML code or can this be delegated to something else?

prabhuramachandran · 2016-11-17T07:24:53Z

Merging for now.

ankitjavalkar force-pushed the partial branch from 4791a49 to c42e68d Compare November 7, 2016 06:55

prathamesh920 requested changes Nov 7, 2016

View reviewed changes

prabhuramachandran reviewed Nov 7, 2016

View reviewed changes

ankitjavalkar added 5 commits November 10, 2016 12:43

dd basic partial marking feature per test case

053e270

Add partial grading to multiple evaluators

0bfa58d

Fix test cases for partial grading feature

576c92b

Add partial grade mark for stdio test case and fix model test cases

4904a83

Add point based weightage for partial grading instead of percentage b…

a09df64

…ased partial grading

ankitjavalkar force-pushed the partial branch from c42e68d to a09df64 Compare November 10, 2016 07:13

Fix minor errors, fix template rendering in grade user

2100ef1

prathamesh920 requested changes Nov 13, 2016

View reviewed changes

prabhuramachandran reviewed Nov 13, 2016

View reviewed changes

ankitjavalkar added 5 commits November 16, 2016 10:58

Fix docstrings, Fix return values of bash evaluator

232c448

Change test_case weightage field name to weight

b32d7e9

Modify docstrings of evaluators

31a15a6

Fix error where answer is correct if last test case is correct

0b15154

Fix Output display of Code Question

cb9a541

Fix test cases to reflect changes in output message

c4a39ba

ankitjavalkar force-pushed the partial branch from 3636257 to c4a39ba Compare November 17, 2016 05:10

prabhuramachandran reviewed Nov 17, 2016

View reviewed changes

ankitjavalkar mentioned this pull request Nov 17, 2016

Multiple Issues with Evaluators #156

Closed

3 tasks

prabhuramachandran merged commit aa6ed71 into FOSSEE:master Nov 17, 2016

ankitjavalkar deleted the partial branch January 16, 2017 09:41

Partial Grading #152

Partial Grading #152

Conversation

ankitjavalkar commented Nov 7, 2016

prathamesh920 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

prabhuramachandran left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ankitjavalkar commented Nov 11, 2016

prathamesh920 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

prabhuramachandran Nov 13, 2016 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ankitjavalkar commented Nov 16, 2016 • edited Loading

ankitjavalkar commented Nov 17, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

prabhuramachandran commented Nov 17, 2016

prabhuramachandran Nov 13, 2016 •

edited

Loading

ankitjavalkar commented Nov 16, 2016 •

edited

Loading