Isolate metadata serialization from device#253
Merged
JieyangChen7 merged 1 commit intoCODARcode:masterfrom Jan 13, 2026
Merged
Conversation
kmorel
force-pushed
the
encapsulate-metadata-serialize
branch
from
122dc94 to
0638c8b
Compare
Previously, the `mgard_x::Metadata` structure had inline code that used protobuf to serialize its values into a buffer. This was problematic because the latest version of protobuf does not compile with nvcc. The issue is a feature of C++17 does not support (and it is unclear if it will ever be supported). Get around this problem by moving (the majority of) the serialization code into the MGARD library and compile it with the host compiler. To make this work, the code now has to serialize into a host buffer and then copy into a device buffer. However, the previous implementation simply copied things one at a time into the device buffer, so this should be no less performant.
kmorel
force-pushed
the
encapsulate-metadata-serialize
branch
from
0638c8b to
8d13307
Compare
Contributor
Author
|
@JieyangChen7 Could you review and (hopefully) merge this change? This is necessary for using recent version of protobuf, which cannot be compiled with nvcc. It basically moves all protobuf references out of Metadata.hpp. |
Contributor
Author
|
@JieyangChen7 Also, this PR fixes issues with the Spack package that we ran into when updating it for the latest release. See spack/spack-packages#2930. The Spack package now pulls the diff from this PR, and in doing so passes all of its tests. (I've set the patch to stop at the next MGARD release in anticipation of this PR being merged by then.) |
Contributor
Author
|
@JieyangChen7 ping |
Collaborator
|
@kmorel Thanks a lot for this PR! I will review this today/tomorrow and merge it this week if everything looks okay. |
Contributor
Author
|
Excellent. Thanks for the review. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Previously, the
mgard_x::Metadatastructure had inline code that used protobuf to serialize its values into a buffer. This was problematic because the latest version of protobuf does not compile with nvcc. The issue is a feature of C++17 does not support (and it is unclear if it will ever be supported).Get around this problem by moving (the majority of) the serialization code into the MGARD library and compile it with the host compiler. To make this work, the code now has to serialize into a host buffer and then copy into a device buffer. However, the previous implementation simply copied things one at a time into the device buffer, so this should be no less performant.
Fixes: #252