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

fix determinant warning #713

Merged
merged 1 commit into from
Feb 23, 2023
Merged

fix determinant warning #713

merged 1 commit into from
Feb 23, 2023

Conversation

tylerflex
Copy link
Collaborator

@tylerflex tylerflex commented Feb 23, 2023

I was getting a lot of divide by zero warnings in the np.linalg.det calls from the mode solver.

Digging into the issue, I found it was because taking a determinant of a complex-valued identity matrix triggers this warning, which was the case for the jacobians in the mode solver that were being passed to np.linalg.det.

import numpy as np
arr = np.eye(3) + 0j
np.linalg.det(arr)
/Users/twhughes/.pyenv/versions/3.10.9/lib/python3.10/site-packages/numpy/linalg/linalg.py:2139: RuntimeWarning: divide by zero encountered in det
  r = _umath_linalg.det(a, signature=signature)
/Users/twhughes/.pyenv/versions/3.10.9/lib/python3.10/site-packages/numpy/linalg/linalg.py:2139: RuntimeWarning: invalid value encountered in det

To fix this, I casted the jacobians tonp.real.

@momchil-flex @weiliangjin2021 @dbochkov-flexcompute is this safe you think? or worth worrying about? I tried filtering the warning but don't know if we want to do that more generally in case we should be worrying about this.

@tylerflex tylerflex changed the base branch from develop to pre/1.9.0 February 23, 2023 03:22
@tylerflex tylerflex linked an issue Feb 23, 2023 that may be closed by this pull request
@tylerflex tylerflex self-assigned this Feb 23, 2023
@tylerflex tylerflex added the 1.9 label Feb 23, 2023
@dbochkov-flexcompute
Copy link
Contributor

hmm, interesting, it doesn't complain on my machine for some reason.

but aren't these jacobians supposed to be always real for our purposes? it seems like they are initialized to be complex because they are copies of complex mu_tensor, but maybe we can just do jac_e = np.real(np.copy(mu_tensor))

@tylerflex tylerflex force-pushed the tyler/fix_det_warning branch 2 times, most recently from 7eda9a5 to ca2afa3 Compare February 23, 2023 14:07
@tylerflex
Copy link
Collaborator Author

If this is the case, then what you suggest would be ideal. I updated the commit to just cast the jacobians to real when they are copied over. Thanks!

@tylerflex tylerflex merged commit 7926cec into pre/1.9.0 Feb 23, 2023
@tylerflex tylerflex deleted the tyler/fix_det_warning branch February 23, 2023 18:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Fix mode solver divide by zero warning
2 participants