Skip to content

Conversation

@Matthew-Whitlock
Copy link
Contributor

@Matthew-Whitlock Matthew-Whitlock commented Apr 24, 2025

Closes #2428

This also enables recovering to the same proxy ID as checkpointed, when possible. Some additional minor convenience features used for resilience are also included, like getting the index from a collection element proxy and getting the objgroup proxy from an objgroup element proxy.

@Matthew-Whitlock Matthew-Whitlock linked an issue Apr 24, 2025 that may be closed by this pull request
@github-actions
Copy link

github-actions bot commented Apr 24, 2025

clang-format output for this changeset:

diff --git a/src/vt/vrt/proxy/collection_elm_proxy.impl.h b/src/vt/vrt/proxy/collection_elm_proxy.impl.h
index b0608db72..586ecd72b 100644
--- a/src/vt/vrt/proxy/collection_elm_proxy.impl.h
+++ b/src/vt/vrt/proxy/collection_elm_proxy.impl.h
@@ -106,8 +106,8 @@ void VrtElmProxy<ColT, IndexT>::serialize(SerT& s) {
         //then immediately deserialize
         using listener::ElementEventEnum;
         listener::ListenFnType<IndexT> m_listener =
-          [*this, buf, listener_id]
-          (ElementEventEnum event, IndexT idx, NodeType) mutable {
+          [*this, buf,
+           listener_id](ElementEventEnum event, IndexT idx, NodeType) mutable {
             if (!(idx == getIndex()))
               return;
             if (event != ElementEventEnum::ElementMigratedIn)

@Matthew-Whitlock Matthew-Whitlock requested a review from nmm0 April 24, 2025 20:32
@Matthew-Whitlock Matthew-Whitlock force-pushed the 2428-checkpoint-using-new-magistrate-trait-based-api branch from 038d3f1 to 7eb4863 Compare April 28, 2025 16:17
@Matthew-Whitlock Matthew-Whitlock force-pushed the 2428-checkpoint-using-new-magistrate-trait-based-api branch from 7eb4863 to 0af1389 Compare April 28, 2025 16:18
@Matthew-Whitlock Matthew-Whitlock force-pushed the 2428-checkpoint-using-new-magistrate-trait-based-api branch from 0af1389 to 4176e68 Compare April 28, 2025 17:29
//We need a traitless serializer, since StoreElmBase uses virtual
//serialization. It doesn't use any traits to specialize serializing
//though, so this is fine.
auto clean_s = s.setTraits();
Copy link
Member

Choose a reason for hiding this comment

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

Is setTraits() actually just getting s with an empty set of traits, or is it mutating s itself? This looks like either mis-use, or mis-naming.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is getting s with an empty set of traits. The traits are immutable, since they're types. I can see how the naming may be misleading, but setTraits() (which gets a serializer with only the specified traits) needs to be distinct from withTraits() and withoutTraits() (which get serializers with the specified traits added-to/removed-from the existing traits). Happy to rename if you have any suggestions, but that'd be a PR in Magistrate

Copy link
Contributor Author

Choose a reason for hiding this comment

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

asTraits() seems a little clearer

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Checkpoint using new Magistrate trait-based API

3 participants