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

Shakemap functions #6624

Merged
merged 5 commits into from
Mar 17, 2021
Merged

Conversation

schmidni
Copy link
Contributor

My results of trying to improve performance of the correlation calculations for shakemaps.

  • changes in cross_corr_matrix are mostlya changing of the if/else orders
  • spatial_covariance_array had a matrix multiplication using for loops, doing a proper matmul using np had a major improvement in performance
  • cholesky function is the one that takes now the longest. I tried switching out all loops with matrix manipulations but could not get a noticeable improvement. Apart from the cholesky decomposition, the L * L.T obviously just takes a fixed amount of time.

Refactor if/else order so that all loops and calculations are skipped if corr='no'
Use numpy function to multiply matrices instead of for loops. Improve performance more than 10x.
@schmidni
Copy link
Contributor Author

Also, there are two more "direct" ways to calculate the LLT matrix in the cholesky function, which however give no performance improvement:

using:
L = numpy.array([numpy.linalg.cholesky(spatial_cov[i]) for i in range(M)]).reshape(N*M, N)

it is possible to get LLT

LLT = (numpy.matmul(L, L.T).reshape(M, N, M, N) * cross_corr[:, None, :, None]).reshape(N*M, N*M)

or

LLT = numpy.multiply( L@L.T, numpy.kron(cross_corr, numpy.ones((N, N))))

Without any direct performance or readability improvement I didn't see a reason to include those changes. I can include them however if you want.

@micheles
Copy link
Contributor

micheles commented Mar 17, 2021

I like this.

@micheles micheles added this to the Engine 3.12.0 milestone Mar 17, 2021
@micheles micheles self-requested a review March 17, 2021 10:49
else:
cross_matrix = numpy.eye(M)
if corr == 'full':
cross_matrix = numpy.full((M, M), 0.99999)
Copy link
Contributor

Choose a reason for hiding this comment

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

We would need a comment here explaining the magic number 0.99999

Copy link
Contributor Author

@schmidni schmidni Mar 17, 2021

Choose a reason for hiding this comment

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

Took the magic number from line 214 of the original code.

I can only guess that there is some numerical reason? "Not quite 1"

Copy link
Contributor

Choose a reason for hiding this comment

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

You are right, it was there from the beginning. Let's me accept the PR as it is, then.

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

Successfully merging this pull request may close these issues.

2 participants