Skip to content

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

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

Merged
merged 15 commits into from
Sep 11, 2023
Merged
Changes from 13 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
42 changes: 40 additions & 2 deletions arithmetic_analysis/jacobi_iteration_method.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@
def jacobi_iteration_method(
coefficient_matrix: NDArray[float64],
constant_matrix: NDArray[float64],
init_val: list[int],
init_val: list[float],
iterations: int,
) -> list[float]:
"""
Expand Down Expand Up @@ -115,6 +115,7 @@ def jacobi_iteration_method(

strictly_diagonally_dominant(table)

"""
# Iterates the whole matrix for given number of times
for _ in range(iterations):
new_val = []
Expand All @@ -130,8 +131,45 @@ def jacobi_iteration_method(
temp = (temp + val) / denom
new_val.append(temp)
init_val = new_val
"""

"""
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.
"""
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@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.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

i_row, i_col = np.where(masks)
ind = i_col.reshape(-1, rows - 1)
"""
'i_col' is converted to a two-dimensional list 'ind',
which will be used to make selections from 'init_val'
('arr' array see below).
"""

# Iterates the whole matrix for given number of times
for _ in range(iterations):
arr = np.take(init_val, ind)
temp = np.sum((-1) * no_diag * arr, axis=1)
new_val = (temp + val_last) / denom
init_val = new_val

return [float(i) for i in new_val]
return new_val.tolist()


# Checks if the given matrix is strictly diagonally dominant
Expand Down