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

simple_neural_network.py: Fewer forward propogations to speed tests #11013

Closed
wants to merge 5 commits into from

Conversation

cclauss
Copy link
Member

@cclauss cclauss commented Oct 26, 2023

Closes #9718

Describe your change:

  • Add an algorithm?
  • Fix a bug or typo in an existing algorithm?
  • Add or change doctests? -- Note: Please avoid changing both code and tests in a single pull request.
  • 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 include at least one URL that points to Wikipedia or another similar explanation.
  • If this pull request resolves one or more open issues then the description above includes the issue number(s) with a closing keyword: "Fixes #ISSUE-NUMBER".

@algorithms-keeper algorithms-keeper bot added enhancement This PR modified some existing files awaiting reviews This PR is ready to be reviewed tests are failing Do not merge until tests pass labels Oct 26, 2023
@tianyizheng02
Copy link
Contributor

At one point I tried to decrease the number of iterations like you did, but it didn't work. 100k iterations isn't even enough for the model to converge within 1 of the true value:

=================================== FAILURES ===================================
______ [doctest] neural_network.simple_neural_network.forward_propagation ______
029 Return the value found after the forward propagation training.
030 
031     >>> res = forward_propagation(32, 100_000)  # Was 10_000_000
032     >>> res > 31 and res < 33
Expected:
    True
Got:
    False

Like I've been saying, this implementation is really suboptimal and should be rewritten.

@quant12345
Copy link
Contributor

quant12345 commented Oct 27, 2023

@cclauss make 1 million and the tests will pass. I tried to measure the time (not in doctest) and it was 9 - 10 times faster.
res = forward_propagation(32, 1_000_000)

performance code:
"""
Forward propagation explanation:
https://towardsdatascience.com/forward-propagation-in-neural-networks-simplified-math-and-code-version-bbcfef6f9250
"""

import math
import random


# Sigmoid
def sigmoid_function(value: float, deriv: bool = False) -> float:
    """Return the sigmoid function of a float.

    >>> sigmoid_function(3.5)
    0.9706877692486436
    >>> sigmoid_function(3.5, True)
    -8.75
    """
    if deriv:
        return value * (1 - value)
    return 1 / (1 + math.exp(-value))


# Initial Value
INITIAL_VALUE = 0.02


def forward_propagation(expected: int, number_propagations: int) -> float:
    """Return the value found after the forward propagation training.

    >>> res = forward_propagation(32, 1_000_000)
    >>> res > 31 and res < 33
    True

    >>> res = forward_propagation(32, 1000)
    >>> res > 31 and res < 33
    False
    """

    # Random weight
    weight = float(2 * (random.randint(1, 100)) - 1)

    for _ in range(number_propagations):
        # Forward propagation
        layer_1 = sigmoid_function(INITIAL_VALUE * weight)
        # How much did we miss?
        layer_1_error = (expected / 100) - layer_1
        # Error delta
        layer_1_delta = layer_1_error * sigmoid_function(layer_1, True)
        # Update weight
        weight += INITIAL_VALUE * layer_1_delta

    return layer_1 * 100


if __name__ == "__main__":
    import doctest

    doctest.testmod()

    import datetime

    now = datetime.datetime.now()
    forward_propagation(32, 10000000)
    old = datetime.datetime.now() - now

    print('old_time', old)

    now = datetime.datetime.now()
    forward_propagation(32, 1000000)
    new = datetime.datetime.now() - now

    print('new_time', new)

    print('old/new', old/new)

Output:

old_time 0:00:07.447717
new_time 0:00:00.735285
old/new 10.129020719856927

@algorithms-keeper algorithms-keeper bot removed the tests are failing Do not merge until tests pass label Oct 27, 2023
@quant12345
Copy link
Contributor

quant12345 commented Oct 27, 2023

@cclauss I just checked 200_000, it also worked. I apologize for not checking it right away.
But, if you try several times, it happens that the tests fail. I'll try to find the optimal number.

@cclauss
Copy link
Member Author

cclauss commented Oct 27, 2023

Thanks, @quant12345 please keep trying to make it faster without becoming a flaky test.

forward_propagation(32, 500_000) # Was 10_000_000 # puts the tests are < 1sec in 4th slowest.

============================= slowest 10 durations =============================
1.32s call     web_programming/get_imdbtop.py::web_programming.get_imdbtop.get_imdb_top_movies
1.21s call     web_programming/fetch_anime_and_play.py::web_programming.fetch_anime_and_play.search_anime_episode_list
1.00s call     web_programming/fetch_anime_and_play.py::web_programming.fetch_anime_and_play.search_scraper
0.90s call     neural_network/simple_neural_network.py::neural_network.simple_neural_network.forward_propagation
0.85s call     web_programming/fetch_anime_and_play.py::web_programming.fetch_anime_and_play.get_anime_episode
0.83s call     dynamic_programming/integer_partition.py::dynamic_programming.integer_partition.partition
0.72s call     graphs/bidirectional_a_star.py::graphs.bidirectional_a_star.AStar
0.50s call     matrix/count_negative_numbers_in_sorted_matrix.py::matrix.count_negative_numbers_in_sorted_matrix.generate_large_matrix
0.37s call     matrix/count_negative_numbers_in_sorted_matrix.py::matrix.count_negative_numbers_in_sorted_matrix.count_negatives_brute_force
0.37s call     dynamic_programming/matrix_chain_multiplication.py::dynamic_programming.matrix_chain_multiplication.matrix_chain_order
================== 1749 passed, 1 warning in 72.96s (0:01:12) ==================

@cclauss
Copy link
Member Author

cclauss commented Oct 27, 2023

@tianyizheng02 For the web_programming jobs, should we consider replacing requests with the plug-compatible httpx and encouraging contributors to use asyncio for web interactions? I doubt it would speed up the tests but it would continue to push contributors to learn new skills. We currently have 35 files that import requests.

@quant12345
Copy link
Contributor

Thanks, @quant12345 please keep trying to make it faster without becoming a flaky test.

forward_propagation(32, 500_000) # Was 10_000_000 # puts the tests are < 1sec in 4th slowest.

Now I tried 400_000 by calling the function 100 times, sometimes the tests fail. At 500_000 I tried 1000, all tests passed(that is, I also think 500_000 is close to the optimal number).

if __name__ == "__main__":
    import doctest

    doctest.testmod()

    for i in range(100):
        doctest.run_docstring_examples(forward_propagation, globals())

@cclauss
Copy link
Member Author

cclauss commented Oct 27, 2023

Nice! Could you please try 450_000 before I merge this?

@quant12345
Copy link
Contributor

quant12345 commented Oct 27, 2023

Nice! Could you please try 450_000 before I merge this?

I run tests 1000 times. It takes some time.

@cclauss

Update: ran it 1000 times with the number 450_000 tests were successful.

@cclauss cclauss enabled auto-merge (squash) October 27, 2023 21:19
@cclauss
Copy link
Member Author

cclauss commented Oct 27, 2023

LGTM. @tianyizheng02

@cclauss cclauss mentioned this pull request Oct 28, 2023
15 tasks
auto-merge was automatically disabled October 28, 2023 22:47

Pull request was closed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
awaiting reviews This PR is ready to be reviewed enhancement This PR modified some existing files
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Speedup our eight slowest pytests (one at a time please)
3 participants