Deep copy from Sidre to Conduit #1510
Conversation
The actual fix is on another branch, but I can't wait for it to be merged.
It's failing. Maybe because non-Umpire allocators are not working right or Group isn't using it right.
Also disable test copying in/out of Umpire. This check needs to wait for proper support for that kind of copy, which is being performed on another branch.
change transfer_allocator method name to transferTo. Also use call-back to return View-specific allocator.
gunney1
force-pushed
the
feature/gunney/copy-sidre-to-conduit
branch
from
6764c24 to
41c15d7
Compare
gunney1
force-pushed
the
feature/gunney/copy-sidre-to-conduit
branch
from
582bbad to
46bb8a7
Compare
src/axom/sidre/core/Group.hpp
Outdated
| * | ||
| * \return pointer to this Group object. | ||
| * | ||
| * This method is recursive. |
There was a problem hiding this comment.
Does recursive mean that the operation is applied to the entire group hierarchy rooted at the group?
If so, maybe it's clearer to say something like that since that is what I see in other comments that do similar hierarchy traversals. If not, I would have to look at the implementation to see what it means...
src/axom/sidre/core/View.cpp
Outdated
| @@ -1872,7 +2100,7 @@ const char* View::getAttributeString(const Attribute* attr) const | |||
| * | |||
| ************************************************************************* | |||
| */ | |||
| int View::getValidAllocatorID(int allocID) | |||
| int View::getValidAxomAllocatorID(int allocID) | |||
There was a problem hiding this comment.
Similar comment/question as earlier about why the Axom qualifier is needed and what it means.
src/axom/sidre/core/View.hpp
Outdated
| * If the state is EMPTY or allocId is the current | ||
| * allocator or is axom::INVALID_ALLOCATOR_ID, this is a no-op. | ||
| * Reallocating an EXTERNAL View means allocating it internally. | ||
| * (This could be revisited, but it is the behavior for now.) |
There was a problem hiding this comment.
I may be missing something, but that seems like the only reasonable choice.
Back out work-arounds that were in place to get View data into the right memory space.
The specification is now done transparently by the hierarchy.
These are obsolete because the hierarchy can now allocate memory in the right allocator id.
gunney1
force-pushed
the
feature/gunney/copy-sidre-to-conduit
branch
from
8e97b84 to
1b67d97
Compare
gunney1
force-pushed
the
feature/gunney/copy-sidre-to-conduit
branch
from
1b67d97 to
634cbaf
Compare
Yes, they are intended even though they're unrelated. With the number of times I had to merge this branch with an active branch that depends on it, it got to be too much work keeping that code separated. |
Summary
Group::deepCopyToConduitandView::deepCopyToConduitViews with different allocators based on theViewtype (string, scalar or array). The mechanism generalizes scalars so they can be array-like but treated as scalars for the purpose of data allocation and copying. In use, this can transparently place big computational data on device and metadata on host.bumpcomponent. We will combine the two after this merge.Fix bug that always allocates string and scalarPreserves the unintended but useful behavior of scalars and strings always being allocated on the host. Scalars and strings now have their own allocator, with the option of being placed on device if the user wants.Viewdata on host. They now allocate in the default allocator of theGroupthat owns theView. (We have discussed this in the project meetings, reaching the concensus was that all data should use the default allocator unless individually overridden, despite the fact that many parts of a hierarchy would only be used on the host.)Manually tell a bunch ofViews to allocate data on the host, where they are used and expected to be.The generalized scalar type is called a "tuple", which is treated like a scalar but may have more than one element. This change:
View::setTuple,View::isTuple,Group::setDefaultArrayAllocatorandGroup::setDefaultTupleAllocatorGroup::setDefaultAllocatorandGroup::getDefaultAllocatorto call the array versions of those methodsThe use of tuples greatly simplifies working with sidre hierarchies. Using the same allocator for all data and manually overriding individual leaves, was not workable.
The goal of this change is toward supporting transparent switching between sidre and conduit. With the addition of
Group::deepCopyToConduit, the state of conduit-sidre hierarchy copies is:* I labeled
importConduitTreeExternalas the shallow conduit-to-sidre copy, but it's not exactly analogous tocreateNativeLayout(the shallow sidre-to-conduit copy).