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

Logger cleanup and clarification #75

Merged
merged 4 commits into from
Jun 1, 2023

Conversation

altheaden
Copy link
Collaborator

@altheaden altheaden commented Jun 1, 2023

This PR changes a few things concerning loggers. Some loggers in run/serial.py have been renamed for better code readability, indicating when they are logging to stdout vs an internal file. The log_filename attribute in Step has been explicitly set to None by default, as opposed to an empty string. And in general, some "compass" wording needed to be changed to "polaris".

Checklist

  • Developer's Guide has been updated
  • Documentation has been built locally and changes look as expected
  • Testing comment in the PR documents testing used to verify the changes

@altheaden altheaden self-assigned this Jun 1, 2023
@altheaden
Copy link
Collaborator Author

I tested this with the pr suite against a baseline. The logging changed as expected and the baseline verification was successful.

@altheaden
Copy link
Collaborator Author

altheaden commented Jun 1, 2023

I also built the documentation locally and it looks as expected.

@altheaden altheaden requested a review from xylar June 1, 2023 02:32
altheaden added 3 commits May 31, 2023 20:43
There are a lot of loggers at play, so this change made them more
readable when they always write to stdout.
Just renamed these older references, and changed the output in the docs
as well.
This is the behavior we expect when checking to see if the step has a log filename later on, such as in `SphericalBaseStep`.
@altheaden
Copy link
Collaborator Author

Oops - I had to rebase, but now it's ready for review :)

Copy link
Collaborator

@xylar xylar left a comment

Choose a reason for hiding this comment

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

This looks wonderful, just 2 small changes.

polaris/run/serial.py Outdated Show resolved Hide resolved
polaris/run/serial.py Outdated Show resolved Hide resolved
@altheaden
Copy link
Collaborator Author

@xylar I fixed the formatting.

Fix string formatting.

Co-authored-by: Xylar Asay-Davis <xylarstorm@gmail.com>
Copy link
Collaborator

@xylar xylar left a comment

Choose a reason for hiding this comment

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

Looks great! I ran cosine_bell with these changes and all the logging details look like what we want.

@xylar xylar merged commit 368ae50 into E3SM-Project:main Jun 1, 2023
@altheaden altheaden deleted the logger-cleanup branch June 1, 2023 22:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants