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

Created triplet_sum in Python/other #2362

Merged
merged 13 commits into from Aug 28, 2020
Merged

Conversation

@Kush1101
Copy link
Contributor

Kush1101 commented Aug 28, 2020

Describe your change:

  • Add an algorithm?
  • Fix a bug or typo in an existing algorithm?
  • Documentation change?

Checklist:

  • I have read CONTRIBUTING.md.
  • This pull request is all my own work -- I have not plagiarized.
  • I know that pull requests will not be merged if they fail the automated tests.
  • This PR only changes one algorithm file. To ease review, please open separate PRs for separate algorithms.
  • All new Python files are placed inside an existing directory.
  • All filenames are in all lowercase characters with no spaces or dashes.
  • All functions and variable names follow Python naming conventions.
  • All function parameters and return values are annotated with Python type hints.
  • All functions have doctests that pass the automated testing.
  • All new algorithms have a URL in its comments that points to Wikipedia or other similar explanation.
  • If this pull request resolves one or more open issues then the commit message contains Fixes: #{$ISSUE_NO}.
@TravisBuddy
Copy link

TravisBuddy commented Aug 28, 2020

Hey @Kush1101,
Something went wrong with the build.

TravisCI finished with status errored, which means the build failed because of something unrelated to the tests, such as a problem with a dependency or the build process itself.

View build log

TravisBuddy Request Identifier: 94a8bd60-e90b-11ea-ae3c-2d16223a9f65
@Kush1101
Copy link
Contributor Author

Kush1101 commented Aug 28, 2020

@cclauss Please review.

@cclauss
Copy link
Member

cclauss commented Aug 28, 2020

OK... With this one we are going to write two different functions in the same file that solve the same problem and then we are going to write a timeit benchmark to tell us which one is faster. The second function should leverage the itertools module (combinations() or permutations()) and should pass the same doctests.

@Kush1101
Copy link
Contributor Author

Kush1101 commented Aug 28, 2020

Okk. Working on it

@Kush1101
Copy link
Contributor Author

Kush1101 commented Aug 28, 2020

OK... With this one we are going to write two different functions in the same file that solve the same problem and then we are going to write a timeit benchmark to tell us which one is faster. The second function should leverage the itertools module (combinations() or permutations()) and should pass the same doctests.

I have written two functions, but I am still not sure about the proper way to include timeit in this code.

def triplet_sum1(arr:List[int],X:int) ->tuple:
    """
    Returns a triplet in in array with sum equal to X,
    else (0, 0, 0).
    >>> triplet_sum1([13, 29, 7, 23, 5], 35)
    (5, 7, 23)
    >>> triplet_sum1([37, 9, 19, 50, 44], 65)
    (9, 19, 37)
    >>> arr = [6, 47, 27, 1, 15]
    >>> summ = 11
    >>> triplet_sum1(arr,summ)
    (0, 0, 0)
    """        
    triplets = list(permutations(arr,3))
    for triplet in triplets:
        if sum(triplet) == X:
            return tuple(sorted(triplet))
    return (0, 0, 0)    
def triplet_sum2(arr: List[int], X: int) -> tuple:
    """
    Returns a triplet in in array with sum equal to X,
    else (0, 0, 0).
    >>> triplet_sum2([13, 29, 7, 23, 5], 35)
    (5, 7, 23)
    >>> triplet_sum2([37, 9, 19, 50, 44], 65)
    (9, 19, 37)
    >>> arr = [6, 47, 27, 1, 15]
    >>> summ = 11
    >>> triplet_sum2(arr,summ)
    (0, 0, 0)
    """
    arr.sort()
    n = len(arr)
    for i in range(n - 1):
        left, right = i + 1, n - 1
        while left < right:
            if arr[i] + arr[left] + arr[right] == X:
                return (arr[i], arr[left], arr[right])
            elif arr[i] + arr[left] + arr[right] < X:
                left += 1
            elif arr[i] + arr[left] + arr[right] > X:
                right -= 1
    else:
        return (0, 0, 0)

Now should I do it like this?

"""
"""
Given an array of integers and another integer X,
we are required to find a triplet from the array such that it's sum is equal to X
"""
from typing import List
from itertools import permutations
from timeit import repeat


def triplet_sum1(arr: List[int], X: int) -> tuple:
    """
    Returns a triplet in in array with sum equal to X,
    else (0, 0, 0).
    >>> triplet_sum1([13, 29, 7, 23, 5], 35)
    (5, 7, 23)
    >>> triplet_sum1([37, 9, 19, 50, 44], 65)
    (9, 19, 37)
    >>> arr = [6, 47, 27, 1, 15]
    >>> summ = 11
    >>> triplet_sum1(arr,summ)
    (0, 0, 0)
    """
    triplets = list(permutations(arr, 3))
    for triplet in triplets:
        if sum(triplet) == X:
            return tuple(sorted(triplet))
    return (0, 0, 0)


def sol1_time():
    setup_code = """ 
from __main__ import triplet_sum1
from random import randint"""
    test_code = """
arr = [randint(-1000, 1000) for i in range(10)]
r = randint(-5000, 5000)
triplet_sum1(arr,r)
    """
    times = repeat(stmt=test_code, setup=setup_code, repeat=5, number=10000)
    return f"Naive solution time is {min(times)}"


def triplet_sum2(arr: List[int], X: int) -> tuple:
    """
    Returns a triplet in in array with sum equal to X,
    else (0, 0, 0).
    >>> triplet_sum2([13, 29, 7, 23, 5], 35)
    (5, 7, 23)
    >>> triplet_sum2([37, 9, 19, 50, 44], 65)
    (9, 19, 37)
    >>> arr = [6, 47, 27, 1, 15]
    >>> summ = 11
    >>> triplet_sum2(arr,summ)
    (0, 0, 0)
    """
    arr.sort()
    n = len(arr)
    for i in range(n - 1):
        left, right = i + 1, n - 1
        while left < right:
            if arr[i] + arr[left] + arr[right] == X:
                return (arr[i], arr[left], arr[right])
            elif arr[i] + arr[left] + arr[right] < X:
                left += 1
            elif arr[i] + arr[left] + arr[right] > X:
                right -= 1
    else:
        return (0, 0, 0)


def sol2_time():
    setup_code = """ 
from __main__ import triplet_sum2
from random import randint"""
    test_code = """
arr = [randint(-1000, 1000) for i in range(10)]
r = randint(-5000, 5000)
triplet_sum2(arr,r)
    """
    times = repeat(stmt=test_code, setup=setup_code, repeat=5, number=10000)
    return f"Optimized solution time is {min(times)}"


if __name__ == "__main__":
    from doctest import testmod

    testmod()
    from random import randint

    arr = [randint(-1000, 1000) for i in range(100)]
    r = randint(-5000, 5000)
    print(f"{triplet_sum1(arr,r)}")
    print(f"{sol1_time()}")
    print(f"{triplet_sum2(arr,r)}")
    print(f"{sol2_time()}")
@Kush1101
Copy link
Contributor Author

Kush1101 commented Aug 28, 2020

I made a PR, so it will be easier for you to review and propose changes. @cclauss

@TravisBuddy
Copy link

TravisBuddy commented Aug 28, 2020

Hey @Kush1101,
Something went wrong with the build.

TravisCI finished with status errored, which means the build failed because of something unrelated to the tests, such as a problem with a dependency or the build process itself.

View build log

TravisBuddy Request Identifier: b60306c0-e918-11ea-ae3c-2d16223a9f65
@cclauss
Copy link
Member

cclauss commented Aug 28, 2020

Nice! A few trailing whitespaces to clean up. When benchmarking it is vital that both algorithms are tested with the exact same dataset so please do something like this...

def make_dataset():
     <your code here>
    return arr, target


dataset = make_dataset()

...

from __main__ import dataset, triplet_sum1, triplet_sum2

...

triplet_sum1(*dataset)

...

triplet_sum2(*dataset)
other/triplet_sum.py Outdated Show resolved Hide resolved
@Kush1101
Copy link
Contributor Author

Kush1101 commented Aug 28, 2020

Nice! A few trailing whitespaces to clean up. When benchmarking it is vital that both algorithms are tested with the exact same dataset so please do something like this...

def make_dataset():
     <your code here>
    return arr, target


dataset = make_dataset()

...

from __main__ import dataset, triplet_sum1, triplet_sum2

...

triplet_sum1(*dataset)

...

triplet_sum2(*dataset)

I will do it now.

Co-authored-by: Christian Clauss <cclauss@me.com>
other/triplet_sum.py Outdated Show resolved Hide resolved
Co-authored-by: Christian Clauss <cclauss@me.com>
other/triplet_sum.py Outdated Show resolved Hide resolved
Co-authored-by: Christian Clauss <cclauss@me.com>
other/triplet_sum.py Outdated Show resolved Hide resolved
Co-authored-by: Christian Clauss <cclauss@me.com>
@TravisBuddy
Copy link

TravisBuddy commented Aug 28, 2020

Hey @Kush1101,
Something went wrong with the build.

TravisCI finished with status errored, which means the build failed because of something unrelated to the tests, such as a problem with a dependency or the build process itself.

View build log

TravisBuddy Request Identifier: 99b5bed0-e928-11ea-ae3c-2d16223a9f65
@TravisBuddy
Copy link

TravisBuddy commented Aug 28, 2020

Hey @Kush1101,
Something went wrong with the build.

TravisCI finished with status errored, which means the build failed because of something unrelated to the tests, such as a problem with a dependency or the build process itself.

View build log

TravisBuddy Request Identifier: bdf40130-e928-11ea-ae3c-2d16223a9f65
@TravisBuddy
Copy link

TravisBuddy commented Aug 28, 2020

Hey @Kush1101,
Something went wrong with the build.

TravisCI finished with status errored, which means the build failed because of something unrelated to the tests, such as a problem with a dependency or the build process itself.

View build log

TravisBuddy Request Identifier: ea5d2d00-e928-11ea-ae3c-2d16223a9f65
@Kush1101
Copy link
Contributor Author

Kush1101 commented Aug 28, 2020

I tried to incorporate all the proposed changes. Both the functions are now tested on the same dataset. I also made other small changes. Please review it now. @cclauss

@Kush1101 Kush1101 requested a review from cclauss Aug 28, 2020
other/triplet_sum.py Outdated Show resolved Hide resolved
other/triplet_sum.py Outdated Show resolved Hide resolved
Co-authored-by: Christian Clauss <cclauss@me.com>
other/triplet_sum.py Outdated Show resolved Hide resolved
other/triplet_sum.py Outdated Show resolved Hide resolved
other/triplet_sum.py Show resolved Hide resolved
other/triplet_sum.py Show resolved Hide resolved
Kush1101 and others added 2 commits Aug 28, 2020
Co-authored-by: Christian Clauss <cclauss@me.com>
Co-authored-by: Christian Clauss <cclauss@me.com>
other/triplet_sum.py Outdated Show resolved Hide resolved
other/triplet_sum.py Show resolved Hide resolved
Kush1101 and others added 2 commits Aug 28, 2020
Co-authored-by: Christian Clauss <cclauss@me.com>
Co-authored-by: Christian Clauss <cclauss@me.com>
@cclauss
Copy link
Member

cclauss commented Aug 28, 2020

This is very impressive. Nice work. This shows the power of an algorithm. The Python standard library gives us tools to express complex things simply -- triplet_sum1() short, tidy, and easy to reason about. However, it is no match of the specific algorithm in triplet_sum2() that is longer and more difficult to reason about but delivers correct answers in a fraction of the time.

Copy link
Member

cclauss left a comment

EXCELLENT!! The benchmark clearly shows the winner!

@cclauss cclauss merged commit 5ef7843 into TheAlgorithms:master Aug 28, 2020
2 checks passed
2 checks passed
codespell
Details
Travis CI - Pull Request Build Passed
Details
@Kush1101
Copy link
Contributor Author

Kush1101 commented Aug 28, 2020

This is very impressive. Nice work. This shows the power of an algorithm. The Python standard library gives us tools to express complex things simply -- triplet_sum1() short, tidy, and easy to reason about. However, it is no match of the specific algorithm in triplet_sum2() that is longer and more difficult to reason about but delivers correct answers in a fraction of the time.

True.

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

Successfully merging this pull request may close these issues.

None yet

3 participants
You can’t perform that action at this time.