Skip to content

Add functionality to print fields in list format.#506

Merged
aaadelmann merged 13 commits into
IPPL-framework:masterfrom
simon-schlepphorst:feature_write_field_in_list_format
May 22, 2026
Merged

Add functionality to print fields in list format.#506
aaadelmann merged 13 commits into
IPPL-framework:masterfrom
simon-schlepphorst:feature_write_field_in_list_format

Conversation

@simon-schlepphorst
Copy link
Copy Markdown
Contributor

For debugging, a feature to print fields in python list or C++ initializer_list compatible format is needed.

e.g. [[0, 1], [2, 3]]

This adds it.

Copy link
Copy Markdown
Collaborator

@aliemen aliemen left a comment

Choose a reason for hiding this comment

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

Nice work, no comments on the print function. Only the BareField function perhaps needs some additional doc, see below.

Please also add a quick unit test for the os << BareField operator you added. Just a small 2x2 field that you fill with some values, print and then compare the printed string to the expected string is probably enough. Can be appended to unit_tests/BareField/BareField.cpp.

Comment thread src/Field/BareField.h
@simon-schlepphorst
Copy link
Copy Markdown
Contributor Author

Should be ready for the final review and merge.

Copy link
Copy Markdown
Collaborator

@aliemen aliemen left a comment

Choose a reason for hiding this comment

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

Test looks good, but I believe the more-than-2-ranks skip is in the wrong place.

Comment thread unit_tests/BareField/BareField.cpp Outdated
Comment thread unit_tests/BareField/BareField.cpp Outdated
@aliemen
Copy link
Copy Markdown
Collaborator

aliemen commented May 21, 2026

cscs-ci run cscs-ci-gh200, cscs-ci-mi300, cscs-ci-openmp

@simon-schlepphorst
Copy link
Copy Markdown
Contributor Author

cscs-ci run cscs-ci-gh200, cscs-ci-mi300, cscs-ci-openmp

Copy link
Copy Markdown
Collaborator

@aliemen aliemen left a comment

Choose a reason for hiding this comment

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

Looks good now. There is a std::cout call that's technically not good in mpi programs, but not a problem here since tests are evaluated with the string stream anyways.

@aaadelmann aaadelmann self-requested a review May 21, 2026 18:17
@aaadelmann
Copy link
Copy Markdown
Member

Something is failing here ...

Copy link
Copy Markdown
Member

@aaadelmann aaadelmann left a comment

Choose a reason for hiding this comment

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

Tests must pass then I can approve

Copy link
Copy Markdown
Collaborator

@aliemen aliemen left a comment

Choose a reason for hiding this comment

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

I think I know what the error is, it's probably a GPU template bug and ViewArgs are not propagated correctly. On GH200 compilation I get the following errors:

  • Incompatible View copy construction
  • View assignment must have compatible spaces
  • View assignment must have compatible layout or have rank <= 1

Basically when passing the Kokkos::View<T**, Kokkos::LayoutRight, ExecSpace> view to BareField<T, Dim, ViewArgs...>::write_as_list, it wants to call detail::write_as_list<T, Dim>(dview_m, out);, but the latter one resolves to/expects const typename ViewType<T, Dim>::view_type& view.

...which is effectively Kokkos ::View<T**> with default layout/space. But dview_m is Kokkos ::View<T**, Kokkos::LayoutRight, ExecSpace>, so Kokkos tries an incompatible view conversion.

I think a quick fix is calling detail::write_as_list<T, Dim, ViewArgs...>(dview_m, out); instead. Alternatively, template detail::write_as_list directly on the view with e.g. template <typename View>.

@simon-schlepphorst
Copy link
Copy Markdown
Contributor Author

Templating directly on the view would be better, but that should be done consistently for all functions in src/Utility/ViewUtils.h.

@aaadelmann aaadelmann self-requested a review May 22, 2026 05:51
@aaadelmann aaadelmann added this pull request to the merge queue May 22, 2026
Merged via the queue into IPPL-framework:master with commit 4010f20 May 22, 2026
6 checks passed
aliemen added a commit that referenced this pull request May 24, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants