-
-
Notifications
You must be signed in to change notification settings - Fork 47.7k
Enhancement of the knapsack algorithm with memorization and generalisation #9295
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
Conversation
for more information, see https://pre-commit.ci
I forgot to add the description of the PR:)
- In
knapsack/knapsack.py, I added a flag - allow_repetition. It can be set to true to allow picking the same item multiple times, and vice versa, only once if it is false. - I refactored the naive recursion function to be more readable and added @lru_cache to optimize the time complexity.
- And simple documentation changes in
knapsack/README.MDfrom 0-1 to 0-N knapsack problem.
I added test cases with allow_repetition = true/false in tests and tests are all passed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A few suggestions, however only two things are blocking an approval from me.
capandcneed to be full names as per the guidelines.- Add a test case that demonstrates that repetition is working.
knapsack/knapsack.py
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Using the lru_cache is a nice improvement. I might be worth exploring adding allow_repitition as a parameter, since lru_cache uses these parameters as a part of its memorization key.
knapsack/knapsack.py
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
cap and c should be given full descriptive names.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You might want to consider a few things before getting into the algorithm:
- Handle any input constraints. Ensure the inputs make sense and raise an exception for the user if they don't
- non-positive numbers
- a capacity lower than the weight of any single object
- etc
- Before getting into the main algorithm, you can go ahead and remove any items that have a weight above the capacity of the knapsack.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The algorithm has been updated to allow repetition. An example/testcase where repetition is used would be appropriate.
4961b3a
into
TheAlgorithms:master
Describe your change:
Checklist: