From 47ce9385e882a60a2acb8bd4e9f72f298b26f451 Mon Sep 17 00:00:00 2001 From: David Mudrak Date: Thu, 13 Jan 2011 14:20:59 +0100 Subject: [PATCH 1/3] MDL-25526 Workshop: fixed random allocation of reviews The patch wraps that foreach ($circles as $circleid => $circle) loop by yet another one for() loop. Reviews are allocated iteratively now. During the first iteration, we try to make sure that at least one circle link exists. During the second iteration, we try to allocate two, etc. Circles are shuffled at the beginning of each iteration. This is supposed to improve the randomness of the allocation. The patch also fixes shuffle_assoc() implementation. The previous implementation actually did not work at all. Also, that removed called to shuffle_assoc() was redundant here. --- mod/workshop/allocation/random/lib.php | 133 +++++++++++++------------ 1 file changed, 69 insertions(+), 64 deletions(-) diff --git a/mod/workshop/allocation/random/lib.php b/mod/workshop/allocation/random/lib.php index 5f65b292cc3..02313e1b561 100644 --- a/mod/workshop/allocation/random/lib.php +++ b/mod/workshop/allocation/random/lib.php @@ -428,76 +428,82 @@ class workshop_random_allocator implements workshop_allocator { // all users will be processed at once $circlegroups = array(0); } - $this->shuffle_assoc($circlegroups); // $o[] = 'debug::circle groups = ' . json_encode($circlegroups); foreach ($circlegroups as $circlegroupid) { $o[] = 'debug::processing circle group id ' . $circlegroupid; $circles = $allcircles[$circlegroupid]; - $this->shuffle_assoc($circles); - foreach ($circles as $circleid => $circle) { - $o[] = 'debug::processing circle id ' . $circleid; - if (!isset($circlelinks[$circleid])) { - $circlelinks[$circleid] = array(); - } - $keeptrying = true; // is there a chance to find a square for this circle? - $failedgroups = array(); // array of groupids where the square should be chosen from (because - // of their group workload) but it was not possible (for example there - // was the only square and it had been already connected - while ($keeptrying && (count($circlelinks[$circleid]) < $numofreviews)) { - // firstly, choose a group to pick the square from - if (NOGROUPS == $gmode) { - if (in_array(0, $failedgroups)) { + // iterate over all circles in the group until the requested number of links per circle exists + // or it is not possible to fulfill that requirment + // during the first iteration, we try to make sure that at least one circlelink exists. during the + // second iteration, we try to allocate two, etc. + for ($requiredreviews = 1; $requiredreviews <= $numofreviews; $requiredreviews++) { + $this->shuffle_assoc($circles); + $o[] = 'debug::iteration ' . $requiredreviews; + foreach ($circles as $circleid => $circle) { + $o[] = 'debug::processing circle id ' . $circleid; + if (!isset($circlelinks[$circleid])) { + $circlelinks[$circleid] = array(); + } + $keeptrying = true; // is there a chance to find a square for this circle? + $failedgroups = array(); // array of groupids where the square should be chosen from (because + // of their group workload) but it was not possible (for example there + // was the only square and it had been already connected + while ($keeptrying && (count($circlelinks[$circleid]) < $requiredreviews)) { + // firstly, choose a group to pick the square from + if (NOGROUPS == $gmode) { + if (in_array(0, $failedgroups)) { + $keeptrying = false; + $o[] = 'error::indent::No more peers available'; // todo translate + break; + } + $targetgroup = 0; + } elseif (SEPARATEGROUPS == $gmode) { + if (in_array($circlegroupid, $failedgroups)) { + $keeptrying = false; + $o[] = 'error::indent::No more peers available in this separate group'; // todo translate + break; + } + $targetgroup = $circlegroupid; + } elseif (VISIBLEGROUPS == $gmode) { + $trygroups = array_diff_key($squaregroupsworkload, array(0 => null)); // all but [0] + $trygroups = array_diff_key($trygroups, array_flip($failedgroups)); // without previous failures + $targetgroup = $this->get_element_with_lowest_workload($trygroups); + } + if ($targetgroup === false) { $keeptrying = false; - $o[] = 'error::indent::No more peers available'; // todo translate + $o[] = 'error::indent::Not enough peers available'; // todo translate break; } - $targetgroup = 0; - } elseif (SEPARATEGROUPS == $gmode) { - if (in_array($circlegroupid, $failedgroups)) { - $keeptrying = false; - $o[] = 'error::indent::No more peers available in this separate group'; // todo translate - break; + $o[] = 'debug::indent::next square should be from group id ' . $targetgroup; + // now, choose a square from the target group + $trysquares = array_intersect_key($squareworkload, $allsquares[$targetgroup]); + // $o[] = 'debug::indent::individual workloads in this group are ' . json_encode($trysquares); + unset($trysquares[$circleid]); // can't allocate to self + $trysquares = array_diff_key($trysquares, array_flip($circlelinks[$circleid])); // can't re-allocate the same + $targetsquare = $this->get_element_with_lowest_workload($trysquares); + if (false === $targetsquare) { + $o[] = 'debug::indent::unable to find an available square. trying another group'; + $failedgroups[] = $targetgroup; + continue; } - $targetgroup = $circlegroupid; - } elseif (VISIBLEGROUPS == $gmode) { - $trygroups = array_diff_key($squaregroupsworkload, array(0 => null)); // all but [0] - $trygroups = array_diff_key($trygroups, array_flip($failedgroups)); // without previous failures - $targetgroup = $this->get_element_with_lowest_workload($trygroups); - } - if ($targetgroup === false) { - $keeptrying = false; - $o[] = 'error::indent::Not enough peers available'; // todo translate - break; - } - $o[] = 'debug::indent::next square should be from group id ' . $targetgroup; - // now, choose a square from the target group - $trysquares = array_intersect_key($squareworkload, $allsquares[$targetgroup]); - // $o[] = 'debug::indent::individual workloads in this group are ' . json_encode($trysquares); - unset($trysquares[$circleid]); // can't allocate to self - $trysquares = array_diff_key($trysquares, array_flip($circlelinks[$circleid])); // can't re-allocate the same - $targetsquare = $this->get_element_with_lowest_workload($trysquares); - if (false === $targetsquare) { - $o[] = 'debug::indent::unable to find an available square. trying another group'; - $failedgroups[] = $targetgroup; - continue; - } - $o[] = 'debug::indent::target square = ' . $targetsquare; - // ok - we have found the square - $circlelinks[$circleid][] = $targetsquare; - $squarelinks[$targetsquare][] = $circleid; - $squareworkload[$targetsquare]++; - $o[] = 'debug::indent::increasing square workload to ' . $squareworkload[$targetsquare]; - if ($targetgroup) { - // recalculate the group workload - $squaregroupsworkload[$targetgroup] = 0; - foreach ($allsquares[$targetgroup] as $squareid => $square) { - $squaregroupsworkload[$targetgroup] += $squareworkload[$squareid]; + $o[] = 'debug::indent::target square = ' . $targetsquare; + // ok - we have found the square + $circlelinks[$circleid][] = $targetsquare; + $squarelinks[$targetsquare][] = $circleid; + $squareworkload[$targetsquare]++; + $o[] = 'debug::indent::increasing square workload to ' . $squareworkload[$targetsquare]; + if ($targetgroup) { + // recalculate the group workload + $squaregroupsworkload[$targetgroup] = 0; + foreach ($allsquares[$targetgroup] as $squareid => $square) { + $squaregroupsworkload[$targetgroup] += $squareworkload[$squareid]; + } + $squaregroupsworkload[$targetgroup] /= count($allsquares[$targetgroup]); + $o[] = 'debug::indent::increasing group workload to ' . $squaregroupsworkload[$targetgroup]; } - $squaregroupsworkload[$targetgroup] /= count($allsquares[$targetgroup]); - $o[] = 'debug::indent::increasing group workload to ' . $squaregroupsworkload[$targetgroup]; - } - } // end of processing this circle - } // end of processing circles in the group + } // end of processing this circle + } // end of one iteration of processing circles in the group + } // end of all iterations over circles in the group } // end of processing circle groups $returned = array(); if (self::USERTYPE_AUTHOR == $numper) { @@ -569,15 +575,14 @@ class workshop_random_allocator implements workshop_allocator { /** * Shuffle the order of array elements preserving the key=>values * - * @author rich at home dot nl - * @link http://php.net/manual/en/function.shuffle.php#80586 * @param array $array to be shuffled * @return true */ protected function shuffle_assoc(&$array) { if (count($array) > 1) { // $keys needs to be an array, no need to shuffle 1 item or empty arrays, anyway - $keys = array_rand($array, count($array)); + $keys = array_keys($array); + shuffle($keys); foreach($keys as $key) { $new[$key] = $array[$key]; } From a1dc301ed5bf7302cb498fbbb3bdaf1856b50375 Mon Sep 17 00:00:00 2001 From: David Mudrak Date: Thu, 13 Jan 2011 17:26:37 +0100 Subject: [PATCH 2/3] MDL-25523 Workshop: not-grouped users are excluded from allocation in visible group mode There was a bug as these users were ignored only as squares but not as circles. This patch makes sure that no new allocation are added to not-grouped users in visible group mode. However, the options 'Remove current allocations' and 'Add self-assessments' apply to them intentionally. Also, the user is warned if there are some not-grouped users found in a workshop in visible groups mode or separate groups mode. --- .../lang/en/workshopallocation_random.php | 2 + mod/workshop/allocation/random/lib.php | 49 ++++++++++++++++++- mod/workshop/allocation/random/styles.css | 4 ++ 3 files changed, 54 insertions(+), 1 deletion(-) create mode 100644 mod/workshop/allocation/random/styles.css diff --git a/mod/workshop/allocation/random/lang/en/workshopallocation_random.php b/mod/workshop/allocation/random/lang/en/workshopallocation_random.php index 2a30f02a81e..59913529c60 100644 --- a/mod/workshop/allocation/random/lang/en/workshopallocation_random.php +++ b/mod/workshop/allocation/random/lang/en/workshopallocation_random.php @@ -33,6 +33,8 @@ $string['assessmentdeleteddetail'] = 'Assessment deallocated: {$a->revie $string['assesswosubmission'] = 'Participants can assess without having submitted anything'; $string['confignumofreviews'] = 'Default number of submissions to be randomly allocated'; $string['noallocationtoadd'] = 'No allocations to add'; +$string['nogroupusers'] = '

Warning: If the workshop is in \'visible groups\' mode or \'separate groups\' mode, then users MUST be part of at least one group to have peer-assessments allocated to them by this tool. Non-grouped users can still be given new self-assessments or have existing assessments removed.

+

These users are currently not in a group: {$a}

'; $string['numofdeallocatedassessment'] = 'Deallocating {$a} assessment(s)'; $string['numofrandomlyallocatedsubmissions'] = 'Randomly allocating {$a} submissions'; $string['numofreviews'] = 'Number of reviews'; diff --git a/mod/workshop/allocation/random/lib.php b/mod/workshop/allocation/random/lib.php index 02313e1b561..761d120c08e 100644 --- a/mod/workshop/allocation/random/lib.php +++ b/mod/workshop/allocation/random/lib.php @@ -178,10 +178,37 @@ class workshop_random_allocator implements workshop_allocator { $this->mform->display(); $out .= ob_get_contents(); ob_end_clean(); - $out .= $output->container_end(); + + // if there are some not-grouped participant in a group mode, warn the user + $gmode = groups_get_activity_groupmode($this->workshop->cm, $this->workshop->course); + if (VISIBLEGROUPS == $gmode or SEPARATEGROUPS == $gmode) { + $users = $this->workshop->get_potential_authors() + $this->workshop->get_potential_reviewers(); + $users = $this->workshop->get_grouped($users); + if (isset($users[0])) { + $nogroupusers = $users[0]; + foreach ($users as $groupid => $groupusers) { + if ($groupid == 0) { + continue; + } + foreach ($groupusers as $groupuserid => $groupuser) { + unset($nogroupusers[$groupuserid]); + } + } + if (!empty($nogroupusers)) { + $list = array(); + foreach ($nogroupusers as $nogroupuser) { + $list[] = fullname($nogroupuser); + } + $a = implode(', ', $list); + $out .= $output->box(get_string('nogroupusers', 'workshopallocation_random', $a), 'generalbox warning nogroupusers'); + } + } + } // TODO $out .= $output->heading(get_string('stats', 'workshopallocation_random')); + $out .= $output->container_end(); + return $out; } @@ -401,6 +428,22 @@ class workshop_random_allocator implements workshop_allocator { } else { throw new moodle_exception('unknownusertypepassed', 'workshop'); } + // get the users that are not in any group. in visible groups mode, these users are exluded + // from allocation by this method + // $nogroupcircles is array (int)$userid => undefined + if (isset($allcircles[0])) { + $nogroupcircles = array_flip(array_keys($allcircles[0])); + } else { + $nogroupcircles = array(); + } + foreach ($allcircles as $circlegroupid => $circles) { + if ($circlegroupid == 0) { + continue; + } + foreach ($circles as $circleid => $circle) { + unset($nogroupcircles[$circleid]); + } + } // $o[] = 'debug::circle links = ' . json_encode($circlelinks); // $o[] = 'debug::square links = ' . json_encode($squarelinks); $squareworkload = array(); // individual workload indexed by squareid @@ -440,6 +483,10 @@ class workshop_random_allocator implements workshop_allocator { $this->shuffle_assoc($circles); $o[] = 'debug::iteration ' . $requiredreviews; foreach ($circles as $circleid => $circle) { + if (VISIBLEGROUPS == $gmode and isset($nogroupcircles[$circleid])) { + $o[] = 'debug::skipping circle id ' . $circleid; + continue; + } $o[] = 'debug::processing circle id ' . $circleid; if (!isset($circlelinks[$circleid])) { $circlelinks[$circleid] = array(); diff --git a/mod/workshop/allocation/random/styles.css b/mod/workshop/allocation/random/styles.css new file mode 100644 index 00000000000..e97804dd488 --- /dev/null +++ b/mod/workshop/allocation/random/styles.css @@ -0,0 +1,4 @@ +.path-mod-workshop .random-allocator .warning { + width: 80%; + margin: 0px auto 15px auto; +} From 241e58e44b1e23bbf153c9cf4df5a082a888ea47 Mon Sep 17 00:00:00 2001 From: David Mudrak Date: Fri, 14 Jan 2011 14:16:10 +0100 Subject: [PATCH 3/3] MDL-25982 Workshop: users from the same group may be excluded from allocation in visible group mode If the Workshop is in the visible group mode, members of the same group can be now optionally excluded from random allocation for a given user. --- .../lang/en/workshopallocation_random.php | 1 + mod/workshop/allocation/random/lib.php | 31 ++++++++++++++++--- .../allocation/random/settings_form.php | 8 +++++ 3 files changed, 36 insertions(+), 4 deletions(-) diff --git a/mod/workshop/allocation/random/lang/en/workshopallocation_random.php b/mod/workshop/allocation/random/lang/en/workshopallocation_random.php index 59913529c60..6a541defdcc 100644 --- a/mod/workshop/allocation/random/lang/en/workshopallocation_random.php +++ b/mod/workshop/allocation/random/lang/en/workshopallocation_random.php @@ -32,6 +32,7 @@ $string['allocationsettings'] = 'Allocation settings'; $string['assessmentdeleteddetail'] = 'Assessment deallocated: {$a->reviewername} is no longer reviewer of {$a->authorname}'; $string['assesswosubmission'] = 'Participants can assess without having submitted anything'; $string['confignumofreviews'] = 'Default number of submissions to be randomly allocated'; +$string['excludesamegroup'] = 'Prevent reviews by peers from the same group'; $string['noallocationtoadd'] = 'No allocations to add'; $string['nogroupusers'] = '

Warning: If the workshop is in \'visible groups\' mode or \'separate groups\' mode, then users MUST be part of at least one group to have peer-assessments allocated to them by this tool. Non-grouped users can still be given new self-assessments or have existing assessments removed.

These users are currently not in a group: {$a}

'; diff --git a/mod/workshop/allocation/random/lib.php b/mod/workshop/allocation/random/lib.php index 761d120c08e..13a41859063 100644 --- a/mod/workshop/allocation/random/lib.php +++ b/mod/workshop/allocation/random/lib.php @@ -76,6 +76,7 @@ class workshop_random_allocator implements workshop_allocator { $o = array(); // list of output messages $numofreviews = required_param('numofreviews', PARAM_INT); $numper = required_param('numper', PARAM_INT); + $excludesamegroup = optional_param('excludesamegroup', false, PARAM_BOOL); $removecurrent = optional_param('removecurrent', false, PARAM_BOOL); $assesswosubmission = optional_param('assesswosubmission', false, PARAM_BOOL); $addselfassessment = optional_param('addselfassessment', false, PARAM_BOOL); @@ -96,7 +97,11 @@ class workshop_random_allocator implements workshop_allocator { } else { $curassessments = $assessments; } - $randomallocations = $this->random_allocation($authors, $reviewers, $curassessments, $numofreviews, $numper, $o); + $options = array(); + $options['numofreviews'] = $numofreviews; + $options['numper'] = $numper; + $options['excludesamegroup'] = $excludesamegroup; + $randomallocations = $this->random_allocation($authors, $reviewers, $curassessments, $o, $options); $newallocations = array_merge($newallocations, $randomallocations); $o[] = 'ok::' . get_string('numofrandomlyallocatedsubmissions', 'workshopallocation_random', count($randomallocations)); unset($randomallocations); @@ -398,19 +403,27 @@ class workshop_random_allocator implements workshop_allocator { * is to connect each "circle" (circles are representing either authors or reviewers) with a required * number of "squares" (the other type than circles are). * + * The passed $options array must provide keys: + * (int)numofreviews - number of reviews to be allocated to each circle + * (int)numper - what user type the circles represent. + * (bool)excludesamegroup - whether to prevent peer submissions from the same group in visible group mode + * * @param array $authors structure of grouped authors * @param resource $reviewers structure of grouped reviewers * @param array $assessments currently assigned assessments to be kept - * @param mixed $numofreviews number of reviews to be allocated to each circle - * @param mixed $numper what user type the circles represent * @param array $o reference to an array of log messages + * @param array $options allocation options * @return array array of (reviewerid => authorid) pairs */ - protected function random_allocation($authors, $reviewers, $assessments, $numofreviews, $numper, &$o) { + protected function random_allocation($authors, $reviewers, $assessments, &$o, array $options) { if (empty($authors) || empty($reviewers)) { // nothing to be done return array(); } + + $numofreviews = $options['numofreviews']; + $numper = $options['numper']; + if (self::USERTYPE_AUTHOR == $numper) { // circles are authors, squares are reviewers $o[] = 'info::Trying to allocate ' . $numofreviews . ' review(s) per author'; // todo translate @@ -514,6 +527,16 @@ class workshop_random_allocator implements workshop_allocator { } elseif (VISIBLEGROUPS == $gmode) { $trygroups = array_diff_key($squaregroupsworkload, array(0 => null)); // all but [0] $trygroups = array_diff_key($trygroups, array_flip($failedgroups)); // without previous failures + if ($options['excludesamegroup']) { + // exclude groups the circle is member of + $excludegroups = array(); + foreach (array_diff_key($allcircles, array(0 => null)) as $exgroupid => $exgroupmembers) { + if (array_key_exists($circleid, $exgroupmembers)) { + $excludegroups[$exgroupid] = null; + } + } + $trygroups = array_diff_key($trygroups, $excludegroups); + } $targetgroup = $this->get_element_with_lowest_workload($trygroups); } if ($targetgroup === false) { diff --git a/mod/workshop/allocation/random/settings_form.php b/mod/workshop/allocation/random/settings_form.php index 4b30f5b3e24..3762e418089 100644 --- a/mod/workshop/allocation/random/settings_form.php +++ b/mod/workshop/allocation/random/settings_form.php @@ -74,6 +74,14 @@ class workshop_random_allocator_form extends moodleform { $mform->setDefault('numper', workshop_random_allocator::USERTYPE_AUTHOR); $mform->addGroup($grpnumofreviews, 'grpnumofreviews', get_string('numofreviews', 'workshopallocation_random'), array(' '), false); + + if (VISIBLEGROUPS == $gmode) { + $mform->addElement('checkbox', 'excludesamegroup', get_string('excludesamegroup', 'workshopallocation_random')); + $mform->setDefault('excludesamegroup', 0); + } else { + $mform->addElement('hidden', 'excludesamegroup', 0); + } + $mform->addElement('checkbox', 'removecurrent', get_string('removecurrentallocations', 'workshopallocation_random')); $mform->setDefault('removecurrent', 0);