dvdoug / BoxPacker

4D bin packing / knapsack problem solver
MIT License
612 stars 156 forks source link

Number of items fit into box off significantly #9

Closed Milow closed 10 years ago

Milow commented 10 years ago

I have a 24x24x24 box. From the test example it would be something like:

$box1 = new TestBox('24x24x24Box', 24, 24, 24, 24, 24, 24, 2, 100);

I have the following 6x6x6 item that weighs 1 pound. Again, from the test example it would be something like:

$item1 = new TestItem('6x6x6Item', 6, 6, 6, 1);

Based on just the dimensions alone, I should be able to fit 64 of that item into the box. The problem that I am having is BoxPacker seems to think it can only fit 16 of this item in to this box. Looking at it, it appears that it fits the depth and length correctly, but the width is not being taken into account? I am not sure.

An example of my current code (simplified for here, of course):

$packer = new Packer();
$packer->addBox(new TestBox('24x24x24Box', 24, 24, 24, 24, 24, 24, 2, 100));

for ($i = 0; $i < 64; $i++) {
    $packer->addItem(new TestItem('6x6x6Item', 6, 6, 6, 1));
}

$packedBoxes = $packer->pack();

I am happy to provide more information if needed. I am using the most recent version (v1.2).

After doing some debugging, I may have found the cause of this. Take the two following lines for example in Packer.php around line 264:

$fitsSameGap = min($remainingWidth - $itemWidth, $remainingLength - $itemLength);
$fitsRotatedGap = min($remainingWidth - $itemLength, $remainingLength - $itemWidth);

In this case, after the first iteration we would have both $fitsSameGap and $fitsRotatedGap = 18. After Three more iterations, we would be down to zero. This fits four items in the box so far. On line 273, in the if statement, on all four of these iterations we enter the first block of the if statement because of this case:

$fitsSameGap <= $fitsRotatedGap

After four iterations, we have filled in the length of the box with four items, which would be correct. On the fifth iteration, both $fitsSameGap and $fitsRotatedGap will now be -6. Because they are both less than or equal to 0, we won't enter the if statement on line 267, and jump down to the else clause. Here, as far as I can tell, it just moves up a layer. After BoxPacker is done, I get the 24x24x24 box filled length wise and depth wise with 16 items (which is right if you don't use width, 4 length by 4 deep). The program does not appear to be taking into account the width of the items.

Hope this helps.

dvdoug commented 10 years ago

Hi

I can reproduce the scenario and confirm the results aren't optimal. Will look into it at my next opportunity.

Thanks, Doug

arischmod commented 10 years ago

Hello everyone, I currently using the dvdoug/BoxPacker (thnx for your effort! )

Im experiment the same problem (the width in not taken onto account).

Is there an easy way of overcoming the thing with the if statement Milow mentioned?? Thnx a lot..

dvdoug commented 10 years ago

I just pushed a fix to master that should now calculate this properly. Please let me know if that resolves it for you - if so, I'll tag and release a new version to Packagist.

Thanks, Doug

arischmod commented 10 years ago

Yes the above problem was solved. Thank you very much for your time..

One thing I have to mention is that (as I see from the logs and code) since the Items get to be placed like:

  1. fill by length
  2. change Row (width)
  3. fill the length... ...
  4. when all the layer floor (bottom) is filled go to new Layer
  5. repeat till there is no Depth left

so All the Items of the same layer considered to have the Depth (depth = hight right??) of the higher Item in the layer. So if for example there are 2 big (full hight) Items places first in a Box, and then we want to fill the remaining space with small Items the SmalItems can not be stackable because the Layer Depth is maximum.

for example if Box (width=4, length=4, depth=4) x 1 we can place BigItem(width=2, length=2, depth=4) x 2 SmalItem(width=1, length=1, depth=1) x 32 so the box is full.

Instead of that the current program will place: BigItem(width=2, length=2, depth=4) x 2 SmalItem(width=1, length=1, depth=1) x 8 the algorithm:

  1. place the 2 BigItems (then the Layer depth will be 4)
  2. fill up the box bottom with 8 SmallItems.
  3. and at this point where in real life we would what to start stacking the remaining SmallItems one on top of the other this is not possible because the 1 layer Depth is 4 (full, because of the BigItems) so the algorithm can not create a new Layer (because it will overreach the roof)

Is that correct??

I'm sorry for the large question but I spend the night trying to figure out what’s happening and why I can not stack the Items. I appreciate a lot what you have done here..

thnx, Aris

dvdoug commented 10 years ago

Great. I've filed issue #11 to handle the problem @arischmod is describing with stacking - that summary correctly describes the current behaviour.

dvdoug commented 10 years ago

Fix for this is in v1.3 which I just tagged.

Milow commented 10 years ago

Just wanted to say thanks for fixing this issue.