Move weight redistibution sanity checking into a pre-check rather tha…

…n a post-check now that the target weight only includes item weight
dvdoug · Feb 2, 2020 · 7b3ebca · 7b3ebca
1 parent 87e2bbc
commit 7b3ebca
Show file tree

Hide file tree

Showing 4 changed files with 44 additions and 59 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -1,5 +1,9 @@
 # Changelog
 
+## [3.5.2] - 2020-02-02
+### Changed
+ - Further optimisation when packing a large number of items
+
 ## [3.5.1] - 2020-01-30
 ### Changed
  - Optimisation when packing a large number of identical items
@@ -89,6 +93,10 @@
 ### Removed
  - HHVM support now that project has a stated goal of no longer targeting PHP7 compatibility
 
+## [2.6.5] - 2020-02-02
+### Changed
+ - Further optimisation when packing a large number of items
+
 ## [2.6.4] - 2020-01-30
 ### Changed
  - Optimisation when packing a large number of identical items
@@ -369,8 +377,9 @@ Initial release
  - Experimental code to get a feel for how calculations can best be implemented
  - Only works if all items fit into a single box (so not production ready at all)
 
-[Unreleased]: https://github.com/dvdoug/BoxPacker/compare/3.5.1...master
+[Unreleased]: https://github.com/dvdoug/BoxPacker/compare/3.5.2...master
 
+[3.5.2]: https://github.com/dvdoug/BoxPacker/compare/3.5.1...3.5.2
 [3.5.1]: https://github.com/dvdoug/BoxPacker/compare/3.5.0...3.5.1
 [3.5.0]: https://github.com/dvdoug/BoxPacker/compare/3.4.1...3.5.0
 [3.4.1]: https://github.com/dvdoug/BoxPacker/compare/3.4.0...3.4.1
@@ -385,6 +394,7 @@ Initial release
 [3.1.0]: https://github.com/dvdoug/BoxPacker/compare/3.0.1...3.1.0
 [3.0.1]: https://github.com/dvdoug/BoxPacker/compare/3.0.0...3.0.1
 [3.0.0]: https://github.com/dvdoug/BoxPacker/compare/2.4.2...3.0.0
+[2.6.5]: https://github.com/dvdoug/BoxPacker/compare/2.6.4...2.6.5
 [2.6.4]: https://github.com/dvdoug/BoxPacker/compare/2.6.3...2.6.4
 [2.6.3]: https://github.com/dvdoug/BoxPacker/compare/2.6.2...2.6.3
 [2.6.2]: https://github.com/dvdoug/BoxPacker/compare/2.6.1...2.6.2

diff --git a/src/WeightRedistributor.php b/src/WeightRedistributor.php
@@ -116,7 +116,7 @@ private function equaliseWeight(PackedBox &$boxA, PackedBox &$boxB, float $targe
         $underWeightBoxItems = $underWeightBox->getItems()->asItemArray();
 
         foreach ($overWeightBoxItems as $key => $overWeightItem) {
-            if ($overWeightItem->getWeight() + $underWeightBox->getItemWeight() > $targetWeight) {
+            if (!static::wouldRepackActuallyHelp($overWeightBoxItems, $overWeightItem, $underWeightBoxItems, $targetWeight)) {
                 continue; // moving this item would harm more than help
             }
 
@@ -142,16 +142,14 @@ private function equaliseWeight(PackedBox &$boxA, PackedBox &$boxB, float $targe
                 continue; //this should never happen, if we can pack n+1 into the box, we should be able to pack n
             }
 
-            if (static::didRepackActuallyHelp($boxA, $boxB, $newHeavierBoxes->top(), $newLighterBoxes->top())) {
-                $this->boxesQtyAvailable[$boxA->getBox()] = $this->boxesQtyAvailable[$boxA->getBox()] + 1;
-                $this->boxesQtyAvailable[$boxB->getBox()] = $this->boxesQtyAvailable[$boxB->getBox()] + 1;
-                $this->boxesQtyAvailable[$newHeavierBoxes->top()->getBox()] = $this->boxesQtyAvailable[$newHeavierBoxes->top()->getBox()] - 1;
-                $this->boxesQtyAvailable[$newLighterBoxes->top()->getBox()] = $this->boxesQtyAvailable[$newLighterBoxes->top()->getBox()] - 1;
-                $underWeightBox = $boxB = $newLighterBoxes->top();
-                $boxA = $newHeavierBoxes->top();
+            $this->boxesQtyAvailable[$boxA->getBox()] = $this->boxesQtyAvailable[$boxA->getBox()] + 1;
+            $this->boxesQtyAvailable[$boxB->getBox()] = $this->boxesQtyAvailable[$boxB->getBox()] + 1;
+            $this->boxesQtyAvailable[$newHeavierBoxes->top()->getBox()] = $this->boxesQtyAvailable[$newHeavierBoxes->top()->getBox()] - 1;
+            $this->boxesQtyAvailable[$newLighterBoxes->top()->getBox()] = $this->boxesQtyAvailable[$newLighterBoxes->top()->getBox()] - 1;
+            $underWeightBox = $boxB = $newLighterBoxes->top();
+            $boxA = $newHeavierBoxes->top();
 
-                $anyIterationSuccessful = true;
-            }
+            $anyIterationSuccessful = true;
         }
 
         return $anyIterationSuccessful;
@@ -178,14 +176,23 @@ private function doVolumeRepack(iterable $items, Box $currentBox): PackedBoxList
      * boxes, or sometimes the box used for the now lighter set of items actually weighs more when empty causing
      * an increase in total weight.
      */
-    private static function didRepackActuallyHelp(PackedBox $oldBoxA, PackedBox $oldBoxB, PackedBox $newBoxA, PackedBox $newBoxB): bool
+    private static function wouldRepackActuallyHelp(array $overWeightBoxItems, Item $overWeightItem, array $underWeightBoxItems, float $targetWeight): bool
     {
-        $oldList = new PackedBoxList();
-        $oldList->insertFromArray([$oldBoxA, $oldBoxB]);
+        $overWeightItemsWeight = array_sum(array_map(static function (Item $item) {return $item->getWeight(); }, $overWeightBoxItems));
+        $underWeightItemsWeight = array_sum(array_map(static function (Item $item) {return $item->getWeight(); }, $underWeightBoxItems));
+
+        if ($overWeightItem->getWeight() + $underWeightItemsWeight > $targetWeight) {
+            return false;
+        }
 
-        $newList = new PackedBoxList();
-        $newList->insertFromArray([$newBoxA, $newBoxB]);
+        $oldVariance = static::calculateVariance($overWeightItemsWeight, $underWeightItemsWeight);
+        $newVariance = static::calculateVariance($overWeightItemsWeight - $overWeightItem->getWeight(), $underWeightItemsWeight + $overWeightItem->getWeight());
 
-        return $newList->getWeightVariance() < $oldList->getWeightVariance();
+        return $newVariance < $oldVariance;
+    }
+
+    private static function calculateVariance(int $boxAWeight, int $boxBWeight)
+    {
+        return ($boxAWeight - (($boxAWeight + $boxBWeight) / 2)) ** 2; //don't need to calculate B and ÷ 2, for a 2-item population the difference from mean is the same for each box
     }
 }
diff --git a/tests/WeightRedistributorTest.php b/tests/WeightRedistributorTest.php
@@ -64,36 +64,4 @@ public function testWeightDistributionWorks(): void
 
         self::assertEquals(0, $packedBoxes->getWeightVariance());
     }
-
-    /**
-     * Test a case where a weight balancing repack is actually 1 box less than before the repack.
-     * Not ideal that this happens, but while it does it can be used for code coverage.
-     */
-    public function testACaseWhereABoxIsEliminated(): void
-    {
-        // first no repack case
-        $packer = new Packer();
-        $packer->setMaxBoxesToBalanceWeight(0);
-        $packer->addBox(new TestBox('Option 1', 230, 300, 240, 160, 230, 300, 240, 15000));
-        $packer->addBox(new TestBox('Option 2', 370, 375, 60, 140, 364, 374, 40, 3000));
-
-        $packer->addItem(new TestItem('Item 1', 220, 310, 12, 679, true), 4);
-        $packer->addItem(new TestItem('Item 2', 210, 297, 5, 242, true), 4);
-
-        $packedBoxes = $packer->pack();
-
-        self::assertCount(3, $packedBoxes);
-
-        // and the repack case
-        $packer = new Packer();
-        $packer->addBox(new TestBox('Option 1', 230, 300, 240, 160, 230, 300, 240, 15000));
-        $packer->addBox(new TestBox('Option 2', 370, 375, 60, 140, 364, 374, 40, 3000));
-
-        $packer->addItem(new TestItem('Item 1', 220, 310, 12, 679, true), 4);
-        $packer->addItem(new TestItem('Item 2', 210, 297, 5, 242, true), 4);
-
-        $packedBoxes = $packer->pack();
-
-        self::assertCount(2, $packedBoxes);
-    }
 }
diff --git a/tests/data/expected.csv b/tests/data/expected.csv
@@ -244,7 +244,7 @@
 0d1983784732e45a55b1040a82db07c1,1,0,48.1,1,0,48.1
 0d36d441ceb0caec57d77547730409a8,2,2601,25.4,1,0,7.6
 0d3c2338df7cc72f5fd90c4b315d4ff0,2,1410156.3,34.8,1,0,21.1
-0d41f4783eeeb20337d3d1f7f058cfe2,2,38025,41.5,1,0,12.4
+0d41f4783eeeb20337d3d1f7f058cfe2,3,333578,27.7,1,0,12.4
 0d66c19661114ec65fb923367d0988fd,1,0,26.5,1,0,26.5
 0d6b0374f997b7d434c9a1dc10da2d46,1,0,32.8,1,0,32.8
 0d786a2440a68c348fa1761614cdf441,1,0,44.3,1,0,44.3
@@ -393,7 +393,7 @@
 1672e9897bfb16f7f644365da8d6992c,1,0,26.4,1,0,26.4
 167db38fa514a30e70f167f70d19f214,1,0,16.2,1,0,16.2
 168065302d10d2f4b683e3d5e7475594,2,536556.3,32.7,1,0,19.8
-1680b70ba864c313d28c9386ce54ac25,4,5476556.5,45.9,3,4493.6,38.5
+1680b70ba864c313d28c9386ce54ac25,4,5476556.5,45.9,3,6570.9,38.5
 1694662b95ac32661ee2b45826bb54f6,1,0,42.4,1,0,42.4
 16a2283358cddda00bf2e47e5ad746d3,1,0,24,1,0,24
 16a59c72d05be053c19e4b32ceafaef0,1,0,29.2,1,0,29.2
@@ -507,7 +507,7 @@
 1bb2791024268994225848fbb9cbc3d3,1,0,35.3,1,0,35.3
 1bb54c648802eecd24f028e8b6142ae7,4,42137.7,42.2,1,0,25.3
 1bc4006c4dd0d71e507869bb2733a0ba,2,1936,31.7,1,0,9.5
-1bd069f31bde829dfe0cd46d82ea770e,2,27060.3,42.3,1,0,12.7
+1bd069f31bde829dfe0cd46d82ea770e,3,313990.2,28.2,1,0,12.7
 1bd0f6a2ed9e36c28d630bbce77c1d61,3,0,41.8,1,0,18.8
 1bde839ae62529764cef435c02aa18fc,1,0,33.6,1,0,33.6
 1be064ca4930df0249218f93619f4cde,1,0,22.8,1,0,22.8
@@ -816,7 +816,7 @@
 2f40c5cd89da22e706e4c2893c2dccb3,1,0,16.4,1,0,16.4
 2f419dab07b92225fa941d15b83f8314,1,0,33.2,1,0,33.2
 2f465e9076de95b96b927208b46705f2,1,0,40.4,1,0,40.4
-2f537fcb5df6f785082d9548928e3d9b,7,1759160.5,41,2,88209,46.7
+2f537fcb5df6f785082d9548928e3d9b,7,1744064.5,41,2,88209,46.7
 2f59969c2f5d7f75ec3b78396d271b87,1,0,38.3,1,0,38.3
 2f5f3f5bdd74b62412f2f6245d78f6ef,1,0,32.6,1,0,32.6
 2f777d3848d7dc6920ae742578dcb053,2,16512.3,30.8,1,0,9.2
@@ -1010,7 +1010,7 @@
 3a37c4a592a4be22a8d8d5e4ee80d7e9,1,0,39.7,1,0,39.7
 3a3dc364bfebd251baf6c465c212ed1c,1,0,15.6,1,0,15.6
 3a3f7d42355f64485a59cb25fd4f6eb2,2,1205604,36.2,1,0,21.9
-3a41ed4b08d3bbc912a4f4aaecee9dd0,3,14816216.2,79.9,3,463611.6,44.4
+3a41ed4b08d3bbc912a4f4aaecee9dd0,3,14816216.2,79.9,3,440547.6,44.4
 3a505cd384cc4eb05934e959fad8185e,2,16900,31.3,1,0,9.4
 3a5f312bc61069dfbada3bea35595687,1,0,38.8,1,0,38.8
 3a8f892aefcf1c1de6b1b126fc6b22e2,1,0,35.1,1,0,35.1
@@ -1037,15 +1037,15 @@
 3bd6564e4bffc131afe76760e02db797,3,7224.9,36.7,1,0,16.5
 3bda69c3fe4e30d6944a02d696661fe3,1,0,29.2,1,0,29.2
 3bdea76452dfd0a2dd349e05688ceea7,1,0,36.2,1,0,36.2
-3bec6daa3c92aab45d0db63b688bad9e,5,90020,34.3,2,3543806.3,18.1
+3bec6daa3c92aab45d0db63b688bad9e,5,89240,34.3,2,3543806.3,18.1
 3becb246809835f192b15039849f3528,1,0,27.5,1,0,27.5
 3bee0a82a9098c92945daac991fca070,2,12.3,23.7,1,0,7.1
 3c1b74fc52f870503d59c3327da6d203,2,607620.3,40,1,0,24.2
 3c29ccbdb900b4ade8f05884d9bafb1c,1,0,38.1,1,0,38.1
 3c2bc39429c8d1a0db6369149f454d65,2,2916,39.5,1,0,11.9
 3c47276e13a1bf073b7f2987f09b21d9,1,0,48,1,0,48
 3c4f92d98c4e7792e46c0269daab7d6d,2,400,28.5,1,0,8.5
-3c51cd3b2b4b213f076ba27e90d04225,3,6253450.7,48.3,2,835396,25.1
+3c51cd3b2b4b213f076ba27e90d04225,3,6251648,48.3,2,835396,25.1
 3c583eb716f36fa0ebe89e3573c733c0,1,0,28.5,1,0,28.5
 3c58eba0522266abecca9b20aad46677,1,0,17.5,1,0,17.5
 3c63070dfe31a39510ed0fdc001cd339,1,0,41.2,1,0,41.2
@@ -2790,7 +2790,7 @@ a3ce775d26ac8be3439ad24b24289f7e,2,87025,25.4,1,0,7.6
 a3dddf09e71c503bc692f33ed332e5d1,1,0,27.5,1,0,27.5
 a3e3c130372bee11d670de991210c9af,1,0,77.1,1,0,77.1
 a3f95b935375913e85f66e3cb6480a9d,1,0,48.5,1,0,48.5
-a403a7d8ed1e7ba344e9a814dc938960,2,1521,44.8,2,1521,32.6
+a403a7d8ed1e7ba344e9a814dc938960,2,1521,44.8,2,1521,44.8
 a413a12acaf8fc5b8cc5118d76c0ebce,1,0,36.5,1,0,36.5
 a4158880f12e267d4120f1c99e90b04b,1,0,5.8,1,0,5.8
 a41d7001551662ca358cf917b4fa71f5,1,0,46.3,1,0,46.3
@@ -2877,7 +2877,7 @@ a8b400d8aef6c488703249becd9b293b,3,18288456.2,62.7,2,11448072.3,32.6
 a8baa80336a3f7799f67d75b56706f88,1,0,26.8,1,0,26.8
 a8ce07dfcf68918f62fe006fdd4e2123,1,0,18.3,1,0,18.3
 a8d355a97fe9359f998901899125564b,1,0,29,1,0,29
-a8df0a7e00e3627fef599719af67e8d0,7,3914818.8,53.1,3,11164.2,29.3
+a8df0a7e00e3627fef599719af67e8d0,7,3914818.8,53.1,3,3914.9,24
 a8f4cfc856ccdba7fac54d706cae29a3,1,0,75.9,1,0,75.9
 a8f9f349cc3a30cb703a9158e52647f3,3,36440.9,31.5,1,0,23.8
 a90a1512baefad9a2c28efde1435ac06,2,961,38.2,1,0,11.4
@@ -3512,7 +3512,7 @@ cee89834047195f849ace58d7900c0b8,1,0,18.5,1,0,18.5
 cefae75274dc3e1ffeaff5635eee0f62,1,0,26.2,1,0,26.2
 cf2c2f8e58511b48193186d0a858d7ff,1,0,33.2,1,0,33.2
 cf3a7710faef1cc4d63d1fda9cdeac25,1,0,31,1,0,31
-cf4d33f33f88d177476780cf0f4d4136,3,71846,39.6,1,0,17.8
+cf4d33f33f88d177476780cf0f4d4136,3,69568.7,39.6,1,0,17.8
 cf52f354e99094e4f1280c80ab97f289,1,0,66.1,1,0,66.1
 cf5d3efee1d5996ff670eccefdd8c8c4,1,0,34.2,1,0,34.2
 cf6c9e450532cd18a8ee0e3186336947,1,0,37.3,1,0,37.3