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.

Sign up for GitHub

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

Clean up for JOSS paper #19

Merged
merged 17 commits into from
Jul 8, 2024
Merged

Clean up for JOSS paper #19

Show file tree
Hide file tree
Changes from 15 commits
Commits
Show all changes
17 commits
Select commit Hold shift + click to select a range
131b03a
Clean-up transpose
ASKabalan Jun 19, 2024
003557c
Clean-up FFT
ASKabalan Jun 19, 2024
6ccdfc9
Clean-up Halo
ASKabalan Jun 19, 2024
32e7929
Make all tests run together
ASKabalan Jun 24, 2024
8de863c
Minor fft fixes
ASKabalan Jun 24, 2024
2f3a8d2
Minor halo fixes
ASKabalan Jun 24, 2024
8d26b00
Minor tranpose fixes
ASKabalan Jun 24, 2024
e1fd558
Fix halo descriptor hashing
ASKabalan Jun 27, 2024
c69b64d
Fix complex slice_unpad problem
ASKabalan Jun 13, 2024
f1e6089
numpy doc and type hinting
ASKabalan Jun 27, 2024
09cc77d
minor fix
ASKabalan Jun 27, 2024
947fd6a
Allow users to build with cuda 11 or 12 (12 by default)
ASKabalan Jun 28, 2024
3b414bb
Update change log
ASKabalan Jul 4, 2024
d7d1308
Update CHANGELOG.md
EiffL Jul 7, 2024
b226590
Remove reduce halo
ASKabalan Jul 7, 2024
5e5484e
Update README.md
EiffL Jul 8, 2024
ac8f2ef
Update README.md
EiffL Jul 8, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 9 additions & 10 deletions CHANGELOG.md
View file
Open in desktop
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No need to get rid of the template, it can guide a future maintainer in the process.

Original file line number Diff line number Diff line change
@@ -1,19 +1,18 @@
# Change log


<!-- Template for documenting changes
## jaxdecomp 0.0.1
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are we really releasing a working code with several new additions as v0.0.1 ?

I would personally prefer something like 0.1.0 and if we are going for https://semver.org/ they would even prefer changing the major version since some of the changes are not backward compatible (to my knowledge).
I know this is a big leap so I would go for 0.1 if you guys agree.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't have enough open source experience to know the implication of a minor vs patch version
But I feel more like this was a minor version rather than a patch
(Then again the major version will always be 0 ..)


* Changes
* Change 1
* Breaking Changes
* Change 1
* Deprecations
* Some things that are getting deprecated
* Bugs
* Bug 1
-->
* New version compatible with JAX 0.4.30
* jaxDecomp now works in a multi-host environment
* Added custom partitioning for FFTs
* Added custom partitioning for halo exchange
* Added custom partitioning for slice_pad and slice_unpad
* Add example for multi-host FFTs in `examples/jaxdecomp_lpt.py`
Comment on lines +11 to +12
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Careful with consistency Added or Add but not both.



## jaxdecomp 0.0.1
## jaxdecomp 0.0.1rc2
* Changes
* Added utility to run autotuning

Expand Down
8 changes: 5 additions & 3 deletions CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,9 @@ cmake_minimum_required(VERSION 3.19...3.25)
set(CMAKE_CXX_STANDARD 17)
set(CMAKE_CUDA_STANDARD 17)
# Latest JAX v0.4.26 no longer supports cuda 11.8
set(NVHPC_CUDA_VERSION 12.2)
# By default, build for CUDA 12.2, users can override this with -DNVHPC_CUDA_VERSION=11.8
set(NVHPC_CUDA_VERSION 12.2 CACHE STRING "CUDA version to build for" )
EiffL marked this conversation as resolved.
Show resolved Hide resolved

# Build debug
# set(CMAKE_BUILD_TYPE Debug)
add_subdirectory(third_party/cuDecomp)
Expand All @@ -15,8 +17,8 @@ option(CUDECOMP_BUILD_FORTRAN "Build Fortran bindings" OFF)
option(CUDECOMP_ENABLE_NVSHMEM "Enable NVSHMEM" OFF)
option(CUDECOMP_BUILD_EXTRAS "Build benchmark, examples, and tests" OFF)

set(CUDECOMP_CUDA_CC_LIST "70;80" CACHE STRING "List of CUDA compute capabilities to build cuDecomp for.")

# 70: Volta, 80: Ampere, 89: RTX 4060
set(CUDECOMP_CUDA_CC_LIST "70;80;89" CACHE STRING "List of CUDA compute capabilities to build cuDecomp for.")
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we already prepare for H100 ?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You are right.
I will add 90


# Add pybind11 and cuDecomp subdirectories
add_subdirectory(pybind11)
Expand Down
3 changes: 1 addition & 2 deletions README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,8 +57,7 @@ with mesh:
# Perform a halo exchange + reduce
exchanged_reduced = jaxdecomp.halo_exchange(padded_array,
EiffL marked this conversation as resolved.
Show resolved Hide resolved
halo_extents=(32,32,32),
halo_periods=(True,True,True),
reduce_halo=True)
halo_periods=(True,True,True))
# Remove the halo regions
recarray = jaxdecomp.slice_unpad(exchanged_reduced, padding_width, pdims)
EiffL marked this conversation as resolved.
Show resolved Hide resolved

Expand Down
11 changes: 7 additions & 4 deletions include/halo.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,10 +22,13 @@ class haloDescriptor_t {
~haloDescriptor_t() = default;

bool operator==(const haloDescriptor_t& other) const {
return (double_precision == other.double_precision && halo_extents == other.halo_extents &&
halo_periods == other.halo_periods && axis == other.axis && config.gdims[0] == other.config.gdims[0] &&
config.gdims[1] == other.config.gdims[1] && config.gdims[2] == other.config.gdims[2] &&
config.pdims[0] == other.config.pdims[0] && config.pdims[1] == other.config.pdims[1]);
return (double_precision == other.double_precision && halo_extents[0] == other.halo_extents[0] &&
halo_extents[1] == other.halo_extents[1] && halo_extents[2] == other.halo_extents[2] &&
halo_periods[0] == other.halo_periods[0] && halo_periods[1] == other.halo_periods[1] &&
halo_periods[2] == other.halo_periods[2] && axis == other.axis &&
config.gdims[0] == other.config.gdims[0] && config.gdims[1] == other.config.gdims[1] &&
config.gdims[2] == other.config.gdims[2] && config.pdims[0] == other.config.pdims[0] &&
config.pdims[1] == other.config.pdims[1]);
Comment on lines +25 to +31
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you put one test per line for readability ?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The formatter does the magic here
Its google I think .. I will look into it

}
};

Expand Down
Loading
Loading