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

BID #140

Merged
merged 31 commits into from
Sep 16, 2024
Merged

BID #140

merged 31 commits into from
Sep 16, 2024

Conversation

acevedo-s
Copy link
Collaborator

@acevedo-s acevedo-s commented Jul 29, 2024

Proposed changes

  1. Routines to calculate the Intrinsic Dimension of binary data (BID) . The routines are independent of the rest of DADApy, and they use extensively cpu-JAX instead of Cython.

  2. With the help of @vdeltatto I added JAX and JAXLIB as a dependencies, and it is only possible to add both for python >=3.8. In the modifications I'm updating the requirement for python>=3.8 in pyproject.toml and I removed the tests for python 3.7

Questions:

  1. Would everyone be fine with requiring python >= 3.8 ? As an example, Ulysses has python 3.8 as the newest python version available in the "module avail" list.

Comments:

  1. JAX-CUDA is quite problematic for version-compatibilities. For JAX-CPU it does not seem to be the case. Meaning: This works fine, but when someone adds something using JAX-CUDA, as @vdeltatto, specific versions for JAX,JAXLIB and python are going to be required.

TODO:

  1. Docstrings are not amazing.
  2. I'd like some opinion about the tutorial, i.e., if it is clear and useful to start using the code.

@acevedo-s acevedo-s marked this pull request as ready for review August 1, 2024 11:40
@acevedo-s acevedo-s requested a review from AldoGl August 1, 2024 11:40
@acevedo-s acevedo-s requested a review from imacocco September 7, 2024 07:13
@imacocco imacocco merged commit 7db1818 into main Sep 16, 2024
8 checks passed
@imacocco imacocco deleted the BID branch September 16, 2024 13:39
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