search

CODARcode / MGARD

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

Use Protobuf in MGARD-X & Add MDR-X #188

Closed JieyangChen7 closed 2 years ago

JieyangChen7 commented 2 years ago

This should be the last major PR before the March release.

JieyangChen7 commented 2 years ago

@ben-e-whitney Please review my changes to mgard.proto and other related changes in the CPU version. Thanks!

ben-e-whitney commented 2 years ago

Looks good to me. Two things:

  1. This is very small, but I suggest we replace NO_SHUFFLE and NOOP in Encoding::Preprocessor and Encoding::Compressor with NOOP_PREPROCESSOR and NOOP_COMPRESSOR, respectively. Then I suggest we replace NOOP in DomainDecomposition::Method with NOOP_METHOD for consistency. (It's the numeric values that matter for the buffers, so we aren't locked in to anything here.)
  2. What is the purpose of the Device message? I want to make sure we don't end up in a situation where, for example, X_HUFFMAN_LZ4 is implemented slightly differently on CUDA than on HIP and so we need to refer to Device to decompress correctly. If we're just storing the backend like you might a timestamp (doesn't change any behavior, but might be good to know), I think that's fine.
JieyangChen7 commented 2 years ago

@ben-e-whitney Thanks for reviewing my changes. I have revised mgard.proto according to your suggestions. The purpose of the Device message is just for letting us know which device (backend) was used for compression. It does not change any behavior. This information might be useful for us to quickly locate a backend that potentially contains a bug. Please let me know if you have any more suggestions. I plan to merge this PR early next week. Thanks!

JieyangChen7 commented 2 years ago

@qliu21 I'm trying to merge this PR. Which merge option do you usually choose Create a merge commit or Rebase and merge?

qliu21 commented 2 years ago

Rebase and merge.

On Tue, Mar 29, 2022 at 9:13 AM Jieyang Chen @.***> wrote:

@qliu21 https://github.com/qliu21 I'm trying to merge this PR. Which merge option do you usually choose Create a merge commit or Rebase and merge?

— Reply to this email directly, view it on GitHub https://github.com/CODARcode/MGARD/pull/188#issuecomment-1081855643, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAYYULGKJJLJONYITDCUDUTVCL6YPANCNFSM5RAYWRIA . You are receiving this because you were mentioned.Message ID: @.***>

-- Dr. Qing Liu Assistant Professor Helen and John C. Hartmann Department of Electrical and Computer Engineering New Jersey Institute of Technology, Newark, NJ Web: https://web.njit.edu/~qliu/ Email: @.*** Phone: 973-596-3526