Checksum warning when autograding fresh generated release version

[artureisele]

Operating system

Ubuntu 20.04.02

nbgrader --version

0.8.5/0.9.5 (warning arises with both versions)

jupyter-lab --version

3.6.8

I create a notebook in the source/task_01 folder with one single grading cell which contains:

# Used for grading. Do not change.
### BEGIN HIDDEN TESTS
assert 0==0
### END HIDDEN TESTS

I run the command: nbgrader generate_assignment task_01
I copy the release version to submitted/test/task_01
I run the command nbgrader autograde task_01

Expected behavior:

Everything runs clear without any warnings or failures

Actual behavior

Everything works correctly but i get the following warning:
[AutogradeApp | WARNING] Attribute 'checksum' for cell cell-e06105f46f78bcd4 has changed! (should be: a78c17a12e10745850cd76fe6b4dc84d, got: 6dc294a55fb4619dbfdb75bdb9c01273)

I digged into your source code and found that when the assignment is generated, the checksum stored in the db file is a different from that stored in the metadata of the release file.

You can see here that the checksum for db is evaluated before the hidden tests are cleared.

And you can see that your code is handling this case by replacing the submitted hidden test with the stored in the db file.

However, this behavior seems not to be intended or did I do something wrong?

I would really like that this warning is only coming if students manipulated the hidden tests.

Thank you very much for any help!

[shreve]

You make a good point. I believe this is the expected behavior and that it has been for quite some time, but the question is: can we do better?

The logic of GenerateAssignment was set a long time ago in e4479be

• Compute the checksums
• Save the checksums to the database
• Clear hidden tests
• Compute the checksums again

This means the release version has the source with hidden tests removed and a checksum representing that source. This makes sense because it allows us to detect if a student has modified the source of the hidden test cell. They shouldn't be allowed to do this, but it is possible.

However the functionality of nbgrader makes this not terribly important because we overwrite the contents of the student-submitted cell anyway.

Because this cell has hidden tests, we expect this to happen and it doesn't deserve to be a warning. I think this behavior of warning when a value needs to be changed makes sense for the other fields: cell_type, locked, points, but not in this case of checksum and source. In the lines above your screenshot, we already set the checksum, whether it's changed or not:

nbgrader/nbgrader/preprocessors/overwritecells.py

Lines 231 to 239 in ca8370f

	# always update the checksum, just in case
	cell.metadata.nbgrader["checksum"] = source_cell.checksum
	# if it's locked, check that the checksum hasn't changed
	if source_cell.locked:
	old_checksum = source_cell.checksum
	new_checksum = utils.compute_checksum(cell)
	if old_checksum != new_checksum:
	self.report_change(grade_id, "checksum", old_checksum, new_checksum)

I see two pathways toward resolving this:

1. Simple -- just remove or downgrade the level of the log line. We are only performing this operation when we're sure this is a graded, locked cell. This saves us from a false positive warning in 99% of cases at the cost of flagging the 1% of cases where a student has tried (unsuccessfully) to manipulate the hidden tests.
2. Careful -- attempt to identify whether the checksum has changed due to this removal of hidden tests. This would involve a bit of architecture design as "whether the cell includes hidden test markers" isn't saved in the database, and the code which can detect that lives outside the OverwriteCells preprocessor.

Unfortunately, I'm not in a position to make a decision on which path to take. @brichet do you have any thoughts?

[finedust]

Any update on this?

I'm afraid that just raising the global log level to avoid that warning would make me lose important warnings somewhere else...
I mean, in the end the grading will be performed correctly regardless of the student attempt, so I don't see the point of issuing a warning for every (even benign) submission, this just creates noise.