Skip to content

Fix type mismatch in eigenvalue case determination and add tests#454

Open
shantanusharma wants to merge 1 commit intoTorchSim:mainfrom
shantanusharma:fix-math-eigenvalues
Open

Fix type mismatch in eigenvalue case determination and add tests#454
shantanusharma wants to merge 1 commit intoTorchSim:mainfrom
shantanusharma:fix-math-eigenvalues

Conversation

@shantanusharma
Copy link

This PR addresses a type mismatch bug in eigenvalue merging

Key changes:
- In `torch_sim/math.py`: Fixed a `TypeError` in `_determine_eigenvalue_case` where boolean masks were incorrectly combined with integer tensors. Added proper device handling for `torch.arange`.
- In `tests/test_math.py`: Added tests for `_determine_eigenvalue_case`.

Checklist:
- [x] Doc strings have been added in the Google docstring format.
- [x] Run ruff on your code.
- [x] Tests have been added for any new functionality or bug fixes.

@shantanusharma
Copy link
Author

Thanks @CompRhys. Following our discussion on #449, I’ve created this clean PR containing only the eigenvalue type-mismatch fix and the associated tests. This should be much easier to review and merge independently.

@CompRhys
Copy link
Member

Thank you for the PR! I think that the issue you are addressing here might be deleted in the larger changes in #439. In that PR in _matrix_log_33 there is a line n_unique = 1 + (diff > num_tol).sum(dim=1) which I believe is the same concept as what you are trying to fix here. Would you be able to look at that PR and see if additional changes are needed?

How did you come across this issue? was it exposed by a simulation error, human code analysis or agentic code analysis?

@shantanusharma
Copy link
Author

Gotcha @CompRhys. Yes. h/t Augment Code agentic code analysis.

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.

2 participants