Update field_utils.hpp

[whannah1]

This adds a check to print_field_hyperslab() to aid in debugging an edge case where a postcondition check tries to throw an error but the underlying grid of the current process is incorrectly defined. Hopefully the check is general enough to help debug other edge cases as well.

[BFB]

[bartgol]

For future reference, the range-based for loops are prob more expressive in this case:

for (auto idx : indices) {
    EKAT_REQUIRE_MSG ( idx>=0 && idx<fl.dim(0),
      "Error! Requested index is invalid.\n"
      "  - field name: " + f.name() + "\n"
      "  - field layout: " + fl.to_string() + "\n"
      "  - requested indices: (" + ekat::join(indices,",") + ")\n");
}

They kind of resemble the python way of looping over iterables.

[tcclevenger]

Added CI approval tag, will merge after testing completes.

[tcclevenger]

Looks like this got caught up in the eamxx/src/share reorg. Now the code you're changing is in the field_utils.cpp file

E3SM/components/eamxx/src/share/field/field_utils.cpp

Line 309 in effa1fb

void print_field_hyperslab (const Field& f,

[whannah1]

Looks like this got caught up in the eamxx/src/share reorg. Now the code you're changing is in the field_utils.cpp file

ah, darn. I'll do a rebase and deal with the conflict.

[bartgol]

Upon further consideration, I think this check is incorrect. We should check that the provided index is in bounds along the dim of the corresponding slice. E.g., if I provide 1 index, I may be slicing away the 2nd or 3rd dim, while the current code checks that the index is in bounds along the 1st dim.

Since the fcn is implemented to use recursion, I think we could simply check that the 1st index is in bounds, by adding a check down below, near these lines

    auto it = ekat::find(layout.tags(),tag);
    EKAT_REQUIRE_MSG (it!=layout.tags().end(),
        "Error! Something went wrong while slicing field.\n"
        "  - field name  : " + f.name() + "\n"
        "  - field layout: " + layout.to_string() + "\n"
        "  - curr tag    : " + e2str(tag) + "\n");
    auto idim = std::distance(layout.tags().begin(),it);

    EKAT_REQUIRE_MSG (idim==0 || idim==1,
        "Error! Cannot subview field for printing.\n"
        "  - field name  : " + f.name() + "\n"
        "  - field layout: " + layout.to_string() + "\n"
        "  - loc tags    : <" + ekat::join(tags,",") + ">\n"
        "  - loc indices : (" + ekat::join(indices,",") + ")\n");

[whannah1]

I'm not really following. Are you saying that there's no way to know if fl.dim(i) corresponds to indices[i]?

In that case I'm not sure how to obtain the size of the correct dimension...

[bartgol]

Say the layout of the field f is {COL,CMP,LEV} and the user is calling print_field_hyperslab(f,{CMP},{3}). Then, in your current impl, you are comparing the first index (3) with the extent of the 0-th component of f, which is not the right one.

[bartgol]

Instead, you should find the first occurrence of tags[i] in the field layout. Something like:

auto it = ekat::find(fl.tags(),tags[i]);
int idx = std::distance(fl.tags().begin(),it);
EKAT_REQUIRE_MSG ( indices[i]>=0 && indices[i]<fl.dim(idx),
  ...

I recon that this is a bit verbose and not so clear. Unfortunately, FieldLayout::dim(FieldTag t), which returns the extent along tag t, requires that the tag appears only once, which may not be the case (e.g., for tensor quantities, or for dyn layouts, where GP appears twice). Perhaps I could change that method so that it returns the extent of the first occurrence of the tag.

Would you like me to do that? Maybe push a commit to this branch?

[whannah1]

How about you handle this update. I'm afraid I don't understand well enough to get it right without your guidance anyway.

[bartgol]

@whannah1 I had to rebase on master, due to conflicts related to the share folder reorg ongoing. If you want to add more stuff, you will have to reset --hard on the current origin version first.

[bartgol]

@mahf708 Since I contributed, it's best if another set of eyes takes a look. Conrad is on vacation until next week, so I pinged you.