Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix extra indentation in fcompare output #3823

Merged
merged 1 commit into from
Mar 19, 2024

Conversation

yut23
Copy link
Member

@yut23 yut23 commented Mar 19, 2024

Summary

This PR fixes the column alignment when the plotfiles being compared have missing variables or NaNs.

The previous code placed the padding spaces from std::setw(50) after the newline, which showed up as extra indentation on the following line. This moves the newlines to a separate string, so the padding only gets applied to the message.

Additional background

Checklist

The proposed changes:

  • fix a bug or incorrect behavior in AMReX
  • add new capabilities to AMReX
  • changes answers in the test suite to more than roundoff level
  • are likely to significantly affect the results of downstream AMReX users
  • include documentation in the code and/or rst files, if appropriate

The previous code placed the padding spaces after the newlines, which
showed up as extra indentation on the following line.
@yut23 yut23 force-pushed the fix_fcompare_indentation branch from 60ae983 to 2a9802c Compare March 19, 2024 17:43
@WeiqunZhang WeiqunZhang merged commit c902981 into AMReX-Codes:development Mar 19, 2024
69 checks passed
@yut23 yut23 deleted the fix_fcompare_indentation branch March 19, 2024 19:23
WeiqunZhang added a commit to WeiqunZhang/regression_testing that referenced this pull request Jun 16, 2024
The function was broken by
AMReX-Codes/amrex#3823. When a variable contains
NaNs, the line used to be "< NaN present >\n", but now it's "< NaN present
>" followed by white spaces and then '\n'. In that case, the line is split
into three fields and we should skip the last field with just white spaces.
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.

2 participants