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

Refactor #37

Merged
merged 16 commits into from
Dec 6, 2023
Merged

Refactor #37

merged 16 commits into from
Dec 6, 2023

Conversation

mavenlin
Copy link
Member

@mavenlin mavenlin commented Dec 6, 2023

Changes:

  • Refactor the repo rule so that it runs much faster.
    The repository rules are quite tricky, originally, the download kept restarting because the labels are not ready, we now explicitly check for all the labels are ready before anything more expensive happens.
  • Add extern C when including the libxc header files, originally on mac and linux the problem is neglected because the symbol names are compatible between c/c++ somehow. On windows this fails.
  • Change the directory names, originally, bazel creates an extra jax_xc folder following the workspace name which kept interfering with the jax_xc package on windows, making it unable to find the real jax_xc package.
  • Properly set the extension to pyd for windows, notice that the extension has to be so for mac & linux and pyd for windows for pybind to work. Therefore, we have to patch the pylibxc code to use the same binary when doing load_library. It is funny that if we have two copies of binary file with different extensions, it causes strange runtime error maybe due to symbol conflict.
  • Add a test for the wheel before uploading them.
  • Windows doesn't provide abi tag, set it to python tag instead.

@mavenlin mavenlin merged commit d265449 into main Dec 6, 2023
4 checks passed
@mavenlin mavenlin deleted the refactor branch December 6, 2023 10:51
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.

1 participant