From: John Ousterhout Date: Thu, 17 Dec 2015 17:46:43 +0000 (-0800) Subject: Updates to grading rubrics X-Git-Url: https://pintos-os.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=6221281f9cf9c0038cafd30714c1bbb847648f74;p=pintos-anon Updates to grading rubrics (make grading a bit kinder and gentler) --- diff --git a/ta-advice/HW1 b/ta-advice/HW1 index 1783dd0..3f2d9ad 100644 --- a/ta-advice/HW1 +++ b/ta-advice/HW1 @@ -18,7 +18,7 @@ at the appropriate time. Details that distinguish solutions include: time in the list, instead of the number of ticks remaining. - Interrupt efficiency: Good solutions that order the list by - wake-up time and store wake-up times (see above two items), + wake-up time and store wake-up times (see above two items) only need their timer interrupt to examine as many items in the list as are actually waking up on this tick (plus one). Otherwise, every sleeping thread must be examined (and @@ -196,25 +196,25 @@ B2: The minimum ASCII art requirement here includes three threads and two locks, e.g. for the sample solution: - /--------------------------\ - | | - /------------------------\ | - Thread H V Thread M V | Thread L | - +-------------+ +-------------+ | +----------------+ | + /--------------------------\ + | | + /--------------------+---\ | + Thread H V Thread M V | Thread L | + +-------------+ +-------------+ | +----------------+ | |priority = 8 | |priority = 8 | | |priority = 8 | | |norm_pri = 8 | |norm_pri = 5 | | |norm_pri = 3 | | |donors = {} | |donors = {H} |-/ |donors = {M} |-/ |donee = M |---->|donee = L |---->|donee = null | - |want_lock = A|--\ |want_lock = B|--\ |want_lock = null| - +-------------+ | +-------------+ | +----------------+ - ^ | ^ ^ | ^ - | Lock A V | | V Lock B | - | +-------------+ | | +-------------+ | - | |holder = M |-/ | |holder = L |-/ - | |waiters = {H}|-\ | |waiters = {M}|-\ - | +-------------+ | | +-------------+ | - | | | | - \------------------/ \------------------/ + |want_lock = A|--\ |want_lock = B|--\ |want_lock = null| + +-------------+ | +-------------+ | +----------------+ + ^ | ^ ^ | ^ + | Lock A V | | V Lock B | + | +-------------+ | | +-------------+ | + | |holder = M |-/ | |holder = L |-/ + | |waiters = {H}|-\ | |waiters = {M}|-\ + | +-------------+ | | +-------------+ | + | | | | + \------------------/ \------------------/ Some explanatory text must also be included. diff --git a/ta-advice/README b/ta-advice/README index 9b68daf..1b5021e 100644 --- a/ta-advice/README +++ b/ta-advice/README @@ -59,7 +59,11 @@ project is roughly this: it by a pristine copy: rm -rf pintos/src/tests - cp -R /usr/class/cs140/pintos/pintos/src/tests pintos/src/tests + cp -R /usr/class/cs140/winter13/pintos/src/tests pintos/src/tests + + (make sure you are using the correct version of Pintos for the current + offering of the course; it may be in a different location than the + example above) 3. Run "make clean" in the top-level directory, to get rid of any binaries or objects mistakenly included in the @@ -87,7 +91,7 @@ project is roughly this: passed, but all yours failed) then you should contact the group. Otherwise, use the grade report that you produced. - Grade reports can vary a number of reasons: QEMU is not + Grade reports can vary for a number of reasons: QEMU is not fully reproducible, Bochs sometimes has reproducibility bugs, the compilers used on different machines may produce code with different behavior, and so on. Finally, it's @@ -125,17 +129,15 @@ There are two parts to grading students' designs: their design documents and their code. Both are lumped into a single grade, taken out of 100 points. -A suggested form to use for grading each project is in hw.txt in -this directory. You should copy this file into each submission -directory and delete the lines that do not apply. - -The subtractions for each kind of problem with a submission are -suggestions. You are free to modify them. You can also add your own -subtractions for problems that are not listed. - -When you add up the subtractions for a project, those for the OVERALL -section are not capped at any particular maximum. Those for -individual problems are capped at the value of the problem. +The form to use for grading each project is in hw.txt in this +directory. You should copy this file into each submission directory +and delete the lines that do not apply. The grading form is divided +into sections, one for each problem, and an OVERALL section. Each +section has its own set of deductions, with a cap on the total +deductions for that section. To compute the overall design score, +first compute the (capped) deductions within each section, then +subtract the section deductions from 100. If the final score is less +than zero, round it up to zero. IMPORTANT: Be critical in grading designs. Most submissions will pass most of the tests, which means that they get almost 50% of the grade @@ -149,6 +151,12 @@ final grades are so low when their intermediate grades seemed so high. It is better to take off lots of points on projects and thereby give students more realistic expectations about their final course grades. +At the same time, don't be unfair. You should only deduct points in +situations where students should have known what was expected of them. +For example, don't invent your own standards for coding style based on +what you think is "right": stick to what is documented, or what any +reasonable person might assume. + Grading Design Documents ------------------------ @@ -160,51 +168,53 @@ Deduct all the points for a given question in these cases: - Missing: The question is not answered at all. - Non-responsive: The response does not actually answer what - is being asked. (If the question does not reasonably apply - to the solution chosen by the group, then the answer should - explain why it does not.) + is being asked. (If the question does not reasonably apply + to the solution chosen by the group, then the answer should + explain why it does not.) - Too long: e.g. a "25 words or less" response takes a whole - page. These qualifiers aim to save the group's time and - your time, so don't waste your time in these cases. + page. These qualifiers aim to save the group's time and + your time, so don't waste your time in these cases. - Too short: The response is evasive or excessively terse to - the point that you don't feel confident in the answer. + the point that you don't feel confident in the answer. - Grossly inaccurate: When you examine the code, you find that - it has no resemblance to the description. + it has no resemblance to the description. - Not implemented: The functionality described in the answer - was not implemented. This often happens when a group runs - out of time before implementing the entire project. Don't - give credit for a design without an implementation. + was not implemented. This often happens when a group runs + out of time before implementing the entire project. Don't + give credit for a design without an implementation. Take off some points (use your judgment) for: + - Conceptual errors: Statements, assumptions, or strong + implications made in the design document are incorrect, + e.g. assuming that unblocking a thread immediately schedules + it. + + - Minor inaccuracies: Some aspects of the code do not match + + - Partial response: Multiple questions are asked, but only + some of them are answered. + +Minor issues (take off only a few points, and only if the problem +is severe): + - Capitalization, punctuation, spelling, or grammar: An - occasional mistake is tolerable, but repeated or frequent - errors should be penalized. Try to distinguish grammar - errors made by non-native speakers of English, which are - understandable, from those made by others, which are less - so. + occasional mistake is tolerable, but repeated or frequent + errors should be penalized. Try to distinguish grammar + errors made by non-native speakers of English, which are + understandable, from those made by others, which are less + so. In Emacs, it is easy to check the spelling of a word: put the cursor on or just after it, then type M-$. You can also make it highlight misspelled words with M-x flyspell-buffer. - - Minor inaccuracies: Some aspects of the code do not match - its description. - - - Conceptual errors: Statements, assumptions, or strong - implications made in the design document are incorrect, - e.g. assuming that unblocking a thread immediately schedules - it. - - - Partial response: Multiple questions are asked, but only - some of them are answered. - - Excessive redundancy: The answer restates much of what is - specified in the assignment. + specified in the assignment. Instructions for recurring questions: @@ -216,15 +226,19 @@ Instructions for recurring questions: less. - Deduct points if the required comment on each declaration is - missing. (The Introduction states "Add a brief comment on - every structure, structure member, global or static - variable, and function definition.") + missing. (The Introduction states "Add a brief comment on + every structure, structure member, global or static + variable, and function definition.") - Deduct points if the response does not describe *purpose*. - We can see the type and the name of each entity from their - declarations. But why are they needed? If the comments - themselves adequately explain purpose, then that is - sufficient. + We can see the type and the name of each entity from their + declarations. But why are they needed? If the comments + themselves adequately explain purpose, then that is + sufficient. Comments should provide information that is not + obvious from the code. A common mistake is for a comment to + duplicate information in the variable or function name. For + example, a variable "pageFaultCount" might have the comment + "count of page faults"; that's not much help! ---- RATIONALE ---- @@ -260,34 +274,34 @@ seconds per page. Nevertheless, you should be able to notice some important flaws: - Inconsistent style: indentation changing randomly between 4 - spaces and 8 spaces per level, between BSD and GNU brace - placement, and so on. (The Introduction states "In new - source files, adopt the existing Pintos style by preference, - but make your code self-consistent at the very least. There - should not be a patchwork of different styles that makes it - obvious that three different people wrote the code.") + spaces and 8 spaces per level, between BSD and GNU brace + placement, and so on. (The Introduction states "In new + source files, adopt the existing Pintos style by preference, + but make your code self-consistent at the very least. There + should not be a patchwork of different styles that makes it + obvious that three different people wrote the code.") - Bad style: such as no indentation at all or cramming many statements - onto a single line. + onto a single line. - Many very long source code lines (over 100 columns wide). - Lack of white space: consistent lack of spaces after commas - or around binary operators that makes code difficult to read. + or around binary operators that makes code difficult to read. - Use of static or file scope ("global") variables instead of - automatic, block scope ("local") variables: one student - submission actually declared 12 (!) different global - variables "just so we don't have to make a new var in each - function". This is unacceptable. + automatic, block scope ("local") variables: one student + submission actually declared 12 (!) different global + variables "just so we don't have to make a new var in each + function". This is unacceptable. - Use of struct thread members instead of automatic, block - scope ("local") variables: sometimes it's not obvious - whether this is the case, but subtract points when it is. + scope ("local") variables: sometimes it's not obvious + whether this is the case, but subtract points when it is. - Code copied into multiple places that should be abstracted - into a function. + into a function. - Gratuitous use of dynamic allocation: e.g. a struct that - contains a pointer to a semaphore instead of a semaphore - itself. + contains a pointer to a semaphore instead of a semaphore + itself. diff --git a/ta-advice/hw1.txt b/ta-advice/hw1.txt index 784c5f1..28c30bf 100644 --- a/ta-advice/hw1.txt +++ b/ta-advice/hw1.txt @@ -17,19 +17,19 @@ OVERALL many statements into one line, other issues at TA's discretion -10 Numerous very long source code lines (> 100 characters) -10 Commented-out or #if'd out code makes real code hard to read - -10 Many missing comments on structure, structure members, - global or static variables, or function definitions + -10 Many missing or uninformative comments on structures, structure + members, global or static variables, or function definitions -10 Function(s) should be decomposed for clarity [indicate function] -10 Cut-and-pasted code should be made into function [indicate where] -10 Uninformative or deceptive identifiers -Subtotal: /10 (not capped) +Total deductions (capped at -40): XXX PROBLEM 1: ALARM CLOCK DOCUMENTATION - -30 Grossly inaccurate: documentation has no resemblance to code - -15 Important inaccuracies: documentation and code differ significantly + -20 Grossly inaccurate: documentation has no resemblance to code + -10 Important inaccuracies: documentation and code differ significantly [indicate how] -5 Minor inaccuracies: documentation and code differ [indicate how] -5 A1: Missing entirely/missing comments or purpose/too long @@ -47,23 +47,23 @@ PROBLEM 1: ALARM CLOCK -10 Interrupt handler always examines or modifies every waiting thread -5 Race between list modification and interrupt handler -10 A timer tick that occurs during list modification delays waking - threads until the next timer tick + threads until the next timer tick -10 Race between two threads modifying list -10 Wakes only one thread even if multiple are finished sleeping -15 Malfunctions (e.g. by busy waiting or not waiting the full time), - even in corner case (e.g. when malloc() returns NULL) + even in corner case (e.g. when malloc() returns NULL) -15 Fixed limit on number of threads that may sleep -5 Uses thread_block() instead of higher-level synchronization primitives -5 Disables interrupts unnecessarily -5 Unnecessary or redundant synchronization in timer_sleep() -Subtotal: /30 (capped at 0) +Total deductions (capped at -30): XXX PROBLEM 2: PRIORITY SCHEDULING DOCUMENTATION - -30 Grossly inaccurate: documentation has no resemblance to code - -15 Important inaccuracies: documentation and code differ significantly + -20 Grossly inaccurate: documentation has no resemblance to code + -10 Important inaccuracies: documentation and code differ significantly [indicate how] -5 Minor inaccuracies: documentation and code differ [indicate how] -5 B1: Missing entirely/missing comments or purpose/too long @@ -95,13 +95,13 @@ PROBLEM 2: PRIORITY SCHEDULING -8 Race in lock_acquire() between priority donation and "down"ing sema -8 Race in lock_release() between release of donated pri and "up"ing sema -Subtotal: /30 (capped at 0) +Total deductions (capped at -30): XXX PROBLEM 3: ADVANCED SCHEDULER DOCUMENTATION - -30 Grossly inaccurate: documentation has no resemblance to code - -15 Important inaccuracies: documentation and code differ significantly + -20 Grossly inaccurate: documentation has no resemblance to code + -10 Important inaccuracies: documentation and code differ significantly [indicate how] -5 Minor inaccuracies: documentation and code differ [indicate how] -5 C1: Missing entirely/missing comments or purpose/too long @@ -126,5 +126,7 @@ PROBLEM 3: ADVANCED SCHEDULER -5 Wastefully recalculates every thread's priority every 4th timer tick -5 Race against timer interrupt in thread_get_recent_cpu() or thread_get_load_avg() + +Total deductions (capped at -30): XXX -Subtotal: /30 (capped at 0) +Total score (100 - deductions): YYY diff --git a/ta-advice/hw2.txt b/ta-advice/hw2.txt index f2764a9..b0ed143 100644 --- a/ta-advice/hw2.txt +++ b/ta-advice/hw2.txt @@ -22,14 +22,14 @@ OVERALL -10 Function(s) should be decomposed for clarity [indicate function] -10 Cut-and-pasted code should be made into function [indicate where] -10 Uninformative or deceptive identifiers - -Subtotal: /10 (not capped) + +Total deductions (capped at -40): XXX PROBLEM 1: ARGUMENT PASSING DOCUMENTATION - -30 Grossly inaccurate: documentation has no resemblance to code - -15 Important inaccuracies: documentation and code differ significantly + -20 Grossly inaccurate: documentation has no resemblance to code + -10 Important inaccuracies: documentation and code differ significantly [indicate how] -5 Minor inaccuracies: documentation and code differ [indicate how] -5 A1: Missing entirely/missing comments or purpose/too long @@ -52,14 +52,14 @@ PROBLEM 1: ARGUMENT PASSING -10 Assumes that the command line is capped at 128 bytes -1 Assumes that individual strings must be word-aligned for performance -10 Argument passing code is difficult to read or poorly abstracted - -Subtotal: /30 (capped at 0) + +Total deductions (capped at -30): XXX PROBLEM 2: SYSTEM CALLS DOCUMENTATION - -60 Grossly inaccurate: documentation has no resemblance to code - -30 Important inaccuracies: documentation and code differ significantly + -40 Grossly inaccurate: documentation has no resemblance to code + -20 Important inaccuracies: documentation and code differ significantly [indicate how] -10 Minor inaccuracies: documentation and code differ [indicate how] -10 B1: Missing entirely/missing comments or purpose/too long @@ -104,5 +104,7 @@ PROBLEM 2: SYSTEM CALLS -10 System call handler is poorly abstracted or unreadable -5 Failed to update comment on process_wait() function after implementing -5 "open" system call fails to release all resources in error cases + +Total deductions (capped at -60): XXX -Subtotal: /60 (capped at 0) +Total score (100 - deductions): YYY diff --git a/ta-advice/hw3.txt b/ta-advice/hw3.txt index 48169d8..59f195e 100644 --- a/ta-advice/hw3.txt +++ b/ta-advice/hw3.txt @@ -22,42 +22,39 @@ OVERALL -10 Function(s) should be decomposed for clarity [indicate function] -10 Cut-and-pasted code should be made into function [indicate where] -10 Uninformative or deceptive identifiers - - EXTRA CREDIT - +5 Sharing - -Subtotal: /10 (not capped) + +Total deductions (capped at -40): XXX PROBLEM 1: PAGE TABLE MANAGEMENT DOCUMENTATION - -25 Grossly inaccurate: documentation has no resemblance to code - -13 Important inaccuracies: documentation and code differ significantly + -16 Grossly inaccurate: documentation has no resemblance to code + -8 Important inaccuracies: documentation and code differ significantly [indicate how] - -5 Minor inaccuracies: documentation and code differ [indicate how] - -5 A1: Missing entirely/missing comments or purpose/too long - -5 A2: Missing/non-responsive/too long/too short - -5 A3: Missing/non-responsive/too long/too short - -5 A3: Sharing implemented, but no explanation of how accessed + -4 Minor inaccuracies: documentation and code differ [indicate how] + -4 A1: Missing entirely/missing comments or purpose/too long + -4 A2: Missing/non-responsive/too long/too short + -4 A3: Missing/non-responsive/too long/too short + -4 A3: Sharing implemented, but no explanation of how accessed and dirty bits are coordinated - -5 A3: Sharing not implemented, but explanation states or implies + -4 A3: Sharing not implemented, but explanation states or implies that a page can have multiple aliases - -5 A4: Missing/non-responsive/too long/too short - -5 A5: Missing/non-responsive/too long/too short + -4 A4: Missing/non-responsive/too long/too short + -4 A5: Missing/non-responsive/too long/too short DESIGN - -30 Not implemented + -25 Not implemented -10 Page dirty status must be determined accurately, not approximately - -Subtotal: /25 (capped at 0) + +Total deductions (capped at -25): XXX PROBLEM 2: PAGING TO AND FROM DISK DOCUMENTATION - -45 Grossly inaccurate: documentation has no resemblance to code - -23 Important inaccuracies: documentation and code differ significantly + -30 Grossly inaccurate: documentation has no resemblance to code + -15 Important inaccuracies: documentation and code differ significantly [indicate how] - -5 Minor inaccuracies: documentation and code differ [indicate how] + -8 Minor inaccuracies: documentation and code differ [indicate how] -5 B1: Missing entirely/missing comments or purpose/too long -5 B2: Missing/non-responsive/too long/too short -5 B3: Missing/non-responsive/too long/too short @@ -84,22 +81,22 @@ PROBLEM 2: PAGING TO AND FROM DISK -10 Clock algorithm implementation resets dirty bits -5 Stack fault heuristic allows more than 32 bytes below stack pointer -3 Stack limit is smaller than 1 MB - -Subtotal: /45 (capped at 0) + +Total deductions (capped at -45): XXX PROBLEM 3: MEMORY-MAPPED FILES DOCUMENTATION - -20 Grossly inaccurate: documentation has no resemblance to code - -10 Important inaccuracies: documentation and code differ significantly + -12 Grossly inaccurate: documentation has no resemblance to code + -6 Important inaccuracies: documentation and code differ significantly [indicate how] - -5 Minor inaccuracies: documentation and code differ [indicate how] - -5 C1: Missing entirely/missing comments or purpose/too long - -5 C2: Missing/non-responsive/too long/too short - -5 C3: Missing/non-responsive/too long/too short - -2 C3: No mention of stack expansion + -3 Minor inaccuracies: documentation and code differ [indicate how] + -3 C1: Missing entirely/missing comments or purpose/too long + -3 C2: Missing/non-responsive/too long/too short + -3 C3: Missing/non-responsive/too long/too short + -3 C3: No mention of stack expansion -2 C3: Claim that memory mappings in different processes can "overlap" - -5 C4: Missing/non-responsive/too long/too short + -3 C4: Missing/non-responsive/too long/too short DESIGN -20 Not implemented @@ -109,5 +106,5 @@ PROBLEM 3: MEMORY-MAPPED FILES -5 Global table of file mappings lacks synchronization -5 Global table allows multiple processes to access same file mapping -5 Superfluous locking on per-thread data structure [which] - -Subtotal: /20 (capped at 0) + +Total deductions (capped at -20): XXX diff --git a/ta-advice/hw4.txt b/ta-advice/hw4.txt index 6ff7e00..30c9f19 100644 --- a/ta-advice/hw4.txt +++ b/ta-advice/hw4.txt @@ -22,14 +22,14 @@ OVERALL -10 Function(s) should be decomposed for clarity [indicate function] -10 Cut-and-pasted code should be made into function [indicate where] -10 Uninformative or deceptive identifiers - -Subtotal: /10 (not capped) + +Total deductions (capped at -40): XXX PROBLEM 1: INDEX AND EXTENSIBLE FILES DOCUMENTATION - -30 Grossly inaccurate: documentation has no resemblance to code - -15 Important inaccuracies: documentation and code differ significantly + -20 Grossly inaccurate: documentation has no resemblance to code + -10 Important inaccuracies: documentation and code differ significantly [indicate how] -5 Minor inaccuracies: documentation and code differ [indicate how] -5 A1: Missing entirely/missing comments or purpose/too long @@ -55,14 +55,14 @@ PROBLEM 1: INDEX AND EXTENSIBLE FILES -5 Global lock for file extension -5 Check on file length followed by obtaining lock fails to recheck length -15 Reads or writes wait for one another, even when file not being extended - -Subtotal: /30 (capped at 0) + +Total deductions (capped at -30): XXX PROBLEM 2: SUBDIRECTORIES DOCUMENTATION - -30 Grossly inaccurate: documentation has no resemblance to code - -15 Important inaccuracies: documentation and code differ significantly + -20 Grossly inaccurate: documentation has no resemblance to code + -10 Important inaccuracies: documentation and code differ significantly [indicate how] -5 Minor inaccuracies: documentation and code differ [indicate how] -5 B1: Missing entirely/missing comments or purpose/too long @@ -85,14 +85,14 @@ PROBLEM 2: SUBDIRECTORIES -5 Use of sector number for current directory has bad semantics when a directory's inode is reused later -10 Operations on different directories must wait for one another - -Subtotal: /30 (capped at 0) + +Total deductions (capped at -30): XXX PROBLEM 3: BUFFER CACHE DOCUMENTATION - -30 Grossly inaccurate: documentation has no resemblance to code - -15 Important inaccuracies: documentation and code differ significantly + -20 Grossly inaccurate: documentation has no resemblance to code + -10 Important inaccuracies: documentation and code differ significantly [indicate how] -5 Minor inaccuracies: documentation and code differ [indicate how] -5 C1: Missing entirely/missing comments or purpose/too long @@ -114,5 +114,5 @@ PROBLEM 3: BUFFER CACHE -5 Creates new thread on most reads or on every read -5 In some cases operations on cached blocks may have to wait for I/O of unrelated blocks to complete - -Subtotal: /30 (capped at 0) + +Total deductions (capped at -30): XXX