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

Revert torch.det for MPS support #132

Closed
wants to merge 2 commits into from

Conversation

tsihyoung
Copy link
Contributor

Summary

Use torch.linalg.cross to suppress deprecated warning raised by torch >= 2.2.

Also, since torch.det utilises LU decomposition to calculate the determinant, I prefer to use the more direct way (i.e., mixed product) to calculate volumes even if torch.det gets MPS support in the future.

Use `torch.linalg.cross` to suppress deprecated warning raised by `torch >= 2.2`.
@janosh
Copy link
Collaborator

janosh commented Feb 28, 2024

thanks @tsihyoung! i think this was superseded by #131? let me know if i missed something.

@janosh janosh closed this Feb 28, 2024
@janosh janosh added the duplicate This issue or pull request already exists label Feb 28, 2024
@tsihyoung
Copy link
Contributor Author

thanks @tsihyoung! i think this was superseded by #131? let me know if i missed something.

I think they are not the same. I replaced torch.cross with torch.linalg.cross, as torch.cross without dim arg is deprecated.

If you prefer to keep torch.cross, then we need torch.cross(lattice[1], lattice[2], dim=-1) to suppress the UserWarning.

@janosh
Copy link
Collaborator

janosh commented Feb 29, 2024

@tsihyoung thanks for pointing that out. i'll change it to torch.linalg.cross in #133

janosh added a commit that referenced this pull request Mar 5, 2024
* use new uv package manager to install deps in CI

* use torch.linalg.cross to silence UserWarning about missing dim kwarg

#132 (comment)

* fix Failed to parse `git+https://gitlab.com/ase/ase`
  Caused by: URL requirement must be preceded by a package name. Add the name of the package before the URL (e.g., `package_name @ https://...`).

* use windows-compat uv install command

* drop git+ from ase install URL

* quotes maybe?

* restore git+

* try ase source install with --upgrade

* generate constraints.txt in CI to prevent uv from downgrading ase installed from git to pypi version

* install ase from git last to prevent uv downgrade
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
duplicate This issue or pull request already exists
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants