search

CODARcode / MGARD

MGARD: MultiGrid Adaptive Reduction of Data
Apache License 2.0
37 stars 25 forks source link

Replace `SIGNATURE` with `mgard::SIGNATURE` #195

Closed ben-e-whitney closed 2 years ago

ben-e-whitney commented 2 years ago

This commit replaces the signature macro defined in include/mgard-x/Metadata.hpp with the mgard::SIGNATURE constant defined in include/format.hpp.

@JieyangChen7, please review this commit. In addition to the signature switch, it also removes the Metadata::signature data member (since the only valid value is mgard::SIGNATURE). I can keep that member if you prefer.

@qliu21, please don't merge this pull request yet. I'll rebase on top of fix-installation-paths once you merge #194.

ben-e-whitney commented 2 years ago

@JieyangChen7, just a reminder to take a quick look at this when you have the time.

JieyangChen7 commented 2 years ago

@JieyangChen7, just a reminder to take a quick look at this when you have the time.

@ben-e-whitney Sorry about the delay. Just got back from vacation. I will review this by the end of this week.

ben-e-whitney commented 2 years ago

No problem, @JieyangChen7. Hope you had a good vacation.

JieyangChen7 commented 2 years ago

@ben-e-whitney I tried to build this PR but it gives me this error with enabling CUDA:

/home/jieyang/dev/MGARD/include/utilities.tpp:248:29: error: ‘__T288’ was not declared in this scope const CartesianProduct<T, N> &iterable, I think this is the NVCC compiler bug we encountered before. Is there any way we can avoid including "utilities.hpp" when including "format.hpp" ?

ben-e-whitney commented 2 years ago

@JieyangChen7 Please try again with the commit I just pushed (b6f53300b87e15c806f71f018fe83bf70c7f6769). On that commit, on Summit, build_scripts/build_mgard_cuda_summit.sh ran successfully for me.

ben-e-whitney commented 2 years ago

Thanks, @JieyangChen7. @qliu21, this is ready to be merged.