jacobi_iteration_method.py the use of vector operations, which reduces the calculation time by dozens of times

[quant12345]

Describe your change:

This is my first pull request, if I did something wrong please let me know.
Replaced loops in jacobi_iteration_method function with vector operations. That gives a reduction in the time for calculating the algorithm.

I am attaching a file test75.py with performance tests (you need the perfplot library).

• 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 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".

test75.py.tar.gz

[quant12345]

How long does the status of awaiting reviews?

[tianyizheng02]

The change mostly looks good to me, but could we get a performance plot with larger input sizes? Based on your current plot, it looks like your new algorithm has runtimes growing at a faster rate than the old algorithm, and my concern is that your algorithm will become slower than the old one once inputs get sufficiently large.

[tianyizheng02]

There shouldn't be such a long comment in the middle of the algorithm. Instead of writing a block comment explaining what each variable does, why not just make the variable names clearer? You could also just add comments to individual lines.

[quant12345]

Corrected.
Double - At the very bottom, I wrote about performance tests with a large array (answered your message).

[tianyizheng02]

Why comment out the old code? Why not just delete it?

[quant12345]

Corrected.

At the very bottom, I wrote about performance tests with a large array (answered your message).

[quant12345]

Just in case, I am attaching the jacobi_simple_two.py file, in which you can run the algorithms once by specifying the array size n = 256 # array size 'coefficient_matrix'. I am attaching data for array sizes of 256 and 500 and the difference only increases 54 and 56 times faster.

n = 256 # array size 'coefficient_matrix'

time_old 0:00:14.445305
time_new 0:00:00.264163
difference in time 54.68469389300262
True

n = 500 # array size 'coefficient_matrix'

time_old 0:01:53.208590
time_new 0:00:01.997707
difference in time 56.66922360714822

Yes, with 'coefficient_matrix' 1000 x 1000 the difference is reduced, but it's still huge. At the same time, the calculation time of the old algorithm is almost 13 minutes. And if you make an array of 2000 x 2000, then the old calculation time of the old algorithm will be approximately: two hours. Need more proof?

n = 1000 # array size 'coefficient_matrix'

time_old 0:12:53.569692
time_new 0:00:17.256253
difference in time 44.828392841454956

jacobi_simple_two.py.tar.gz

[quant12345]

The change mostly looks good to me, but could we get a performance plot with larger input sizes? Based on your current plot, it looks like your new algorithm has runtimes growing at a faster rate than the old algorithm, and my concern is that your algorithm will become slower than the old one once inputs get sufficiently large.

Attached in the archived file 'test75.py.tar.gz', file 'test75.py'. It contains the 'jacobi_test' function, which calculates according to the new algorithm. The arrays 'coefficient', 'constant', 'init_val' and the number 'n' (ascending) are generated. In the first iteration 2 x 2, the last iteration we have an array of 256 x 256( 'coefficient').
To check the performance, the perfplot library is used, in which, in the 'equality_check' function, both received arrays are checked for equality.

The size of the 'coefficient' array is printed on each iteration:

After n > 256 the array becomes so much larger and np.inf and -np.inf appear, which leads to nan numbers, both in the new and old calculation(this happens because the values get very small. With the upgraded numpy this doesn't happen).

[quant12345]

It is not clear that my pull request is not needed, I seem to have done everything that was asked?

[rohan472000]

@tianyizheng02 @cclauss , it looks good to me, if you also thinks the same, then better to proceed with some actions on this PR.

[cclauss]

The triple-quoted strings confuse the reader about what is code and what is comments.

Suggested change

	"""
	denom - a list of values along the diagonal
	"""
	denom = np.diag(coefficient_matrix)
	"""
	val_last - values of the last column of the table array
	"""
	val_last = table[:, -1]
	"""
	masks - boolean mask of all strings without diagonal
	elements array coefficient_matrix
	"""
	masks = ~np.eye(coefficient_matrix.shape[0], dtype=bool)
	"""
	no_diag - coefficient_matrix array values without diagonal elements
	"""
	no_diag = coefficient_matrix[masks].reshape(-1, rows - 1)
	"""
	Here we get 'i_col' - these are the column numbers, for each row
	without diagonal elements, except for the last column.
	"""
	# denom - a list of values along the diagonal
	denom = np.diag(coefficient_matrix)
	# val_last - values of the last column of the table array
	val_last = table[:, -1]
	# masks - boolean mask of all strings without diagonal elements array coefficient_matrix
	masks = ~np.eye(coefficient_matrix.shape[0], dtype=bool)
	# no_diag - coefficient_matrix array values without diagonal elements
	no_diag = coefficient_matrix[masks].reshape(-1, rows - 1)
	# Here we get 'i_col' - these are the column numbers, for each row
	# without diagonal elements, except for the last column.

[cclauss]

Please use real variable names: denom -> denominator, no_diag --> no_diagonals (some readers could assume no_diagnostics). Is there a better variable name than temp? Using real names will make the code more self-documenting and will mean that fewer comments will be required to make things clear to your reader.

[quant12345]

@cclauss changed variable names. But temp is a multifaceted task, I made this name not too long, if everything is described in its name it will turn out to be a completely cumbersome name. Replaced comments with single-row ones.

Why did I use multi-line comments in the previous version: since not everything fit, and I didn’t want to comment several lines with single-line comments.

[cclauss]

# There is nothing wrong with really long comments if they are appropriately wrapped at
# 88 characters per line.