Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bug + fix found in reference implementation #26

Closed
MarkusFaatz opened this issue Jan 31, 2021 · 3 comments
Closed

Bug + fix found in reference implementation #26

MarkusFaatz opened this issue Jan 31, 2021 · 3 comments

Comments

@MarkusFaatz
Copy link

Hey there!
I recently searched for the EB-AFIT algorithm and I found this bug in the original C-Code project that seems to have also found his way to this C# project.

As Robertson also suggests a fix you might have a look at these two lines in your code

smallestZ.CumX = smallestZ.CumX - cboxx;
itemsToPack[cboxi].CoordX = smallestZ.CumX - cboxx;

where it might be a mistake to decrement cboxx twice from the itemsToPack[cboxi].CoordX

@davidmchapman
Copy link
Owner

Very interesting! I'm open to updating the code. I've taken time away from this project but now have some more time to hop back into it. Soon I will run the tests on the before and after code to see how this affects them. I'll report back.

Thank you for bringing this up!

@davidmchapman
Copy link
Owner

I was able to verify both the issue and the proposed fix.

Following the original comment from @garrettsickles, I ran the packing algorithm on set 4 of the randomly generated problem sets from Appendix D of the original master's thesis. This test attempts to pack 1000+ items into a single 104x96x84 container.

Packing with the unfixed code yielded the same issue @garrettsickles noted, shown here:
box94-1

Notably, you can also see where the accidental shifting started in this layer. That's what pushed the final item in the layer outside the bounds of the container:
box94-2

With the proposed fix, the algorithm correctly closes that gap:
box94-fixed

Great work, @garrettsickles, @robertson-so, and @MarkusFaatz!!

I will roll the fix into master in this repo soon. Thanks, everyone!

@davidmchapman
Copy link
Owner

Fix committed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants