search

flatironinstitute / cufinufft

Nonuniform fast Fourier transforms of types 1 and 2, in 1D, 2D, and 3D, on the GPU
Other
83 stars 18 forks source link

More general .gitignore #110

Closed tianrluo closed 3 years ago

tianrluo commented 3 years ago

Dear authors,

I made some minor changes to make .gitignore more general, and include a way to keep the setting of LD_LIBRARY_PATH local to a certain conda environment.

Hopefully this can be helpful.

Thanks!

tianrluo commented 3 years ago

cupy is needed in the CI testing environment to assess the PR. Right now the auto test is failing

janden commented 3 years ago

Thank you for these contributions @tianrluo. Since this PR incorporates several unrelated changes, it is hard to review as is. I therefore suggest you split it into smaller, logically consistent PRs that we can review one at a time. Furthermore, it is good practice to anchor non-trivial changes (such as adding cupy support) through discussion in an Issue to ensure that there is agreement on the proposed change before proceeding to making such changes to the code.

tianrluo commented 3 years ago

@janden Thanks for the suggestion! I have rolled back the commit that adds cupy support for the moment, and I am creating a new issue for discussing adding cupy support. Thanks!

janden commented 3 years ago

Again, please split this commit into smaller ones. We cannot review all these changes wholesale. From what I can see, there are a few distinct changes proposed here:

Please separate these into different PRs with explanations and motivations for each.

tianrluo commented 3 years ago

@janden

Thanks for the suggestion! I have rolled back the branch.

This commit makes .gitignore more automatic in some sense at ignoring unwanted changes. I felt this was needed when I installed the package by compiling the codes and doing a local pip install.