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

Add automatic logging #479

Merged
merged 22 commits into from
Mar 21, 2024
Merged

Add automatic logging #479

merged 22 commits into from
Mar 21, 2024

Conversation

MakisH
Copy link
Member

@MakisH MakisH commented Mar 12, 2024

When users ask for help, or simply when debugging any kind of simulation, it is very convenient to look at full log files in a text editor/viewer, instead of looking through short snippets of screen output (or, even worse, a screenshot of that).

This PR uses tee to log the screen output of each step in each run.sh file into a log file (e.g., fluid-openfoam.log).

  • the first call opens the file, subsequent calls append to that
  • errors are redirected
  • ansi color codes are written without any special treatment. Somehow, my terminal/OpenFOAM does not colorize those anyway when using the unmodified run.sh.
  • the clean.sh script cleans this.

The touch case.foam is now moved to run-openfoam.sh.

While we could essentially derive the name of the directory automatically, I could not find a short and POSIX-compliant way to do that. @fsimonis any suggestion? ✔️

I will apply the changes to everything once we agree on a standard. ✔️

TODO (but not necessary to incorporate before merging, this is already covering the vast majority of the cases):

  • ASTE
  • run-dealii.sh

@MakisH MakisH added this to the v202403.0 milestone Mar 12, 2024
@MakisH MakisH requested a review from fsimonis March 12, 2024 08:20
@uekerman
Copy link
Member

Does this eat up all terminal output or simply duplicate things?
I would not want to lose terminal output.

@MakisH
Copy link
Member Author

MakisH commented Mar 12, 2024

It only duplicates. The user experience remains exactly the same, it only duplicates information into log files.

@fsimonis
Copy link
Member

If you need the directory of the current script, then you can do this:

  1. get the full path of the file on disk
  2. get the full directory
  3. get the last part of the path
DIR="$(basename $(dirname $(readlink -f "$0")))"

or using xargs

DIR="$(readlink -f "$0" | xargs dirname | xargs basename)"

@MakisH
Copy link
Member Author

MakisH commented Mar 12, 2024

@fsimonis Thank you, I used the xargs option.

I simplified a lot, and now I am also printing date in the beginning and end of the output (these logs anyway contain timestamps, so comparisons are not an issue).

Some considerations:

  • date uses the locale-originating format: Di 12. Mär 19:09:38 CET 2024. I wonder how this could behave in test systems with no locale enabled (do we have any?).
  • if I open the logs in less and hit Ctrl+End, it somehow shows Waiting for data... (interrupt to abort), in which case I have to first hit Ctrl+C and then q to exit. I wonder if this is an issue with tee, with less, or anything else.

Any thought/suggestions/objections?

@fsimonis
Copy link
Member

date uses the locale-originating format: Di 12. Mär 19:09:38 CET 2024. I wonder how this could behave in test systems with no locale enabled (do we have any?).

Pragmatic solution: data --rfc-email produces the format Wed, 13 Mar 2024 10:14:04 +0100.

if I open the logs in less and hit Ctrl+End, it somehow shows Waiting for data... (interrupt to abort), in which case I have to first hit Ctrl+C and then q to exit. I wonder if this is an issue with tee, with less, or anything else.

less has issues if the file is still being written to. As you use tee, it is possible that the solver is still alive and the pipe forwarding to the file still intact.

@MakisH
Copy link
Member Author

MakisH commented Mar 15, 2024

I added now formatted start time, end time, and duration:

End

Cleaning up any time directories without results
Done.
Started on:  Fri, 15 Mar 2024 08:00:05 +0100
Finished on: Fri, 15 Mar 2024 08:00:16 +0100
Duration:    11 seconds

I understand that the way to stop the "waiting for data" in less is to "unbuffer" the output, which I assume could have significant performance impact (I have not measured).

While I would prefer to solve this, I am also fine if it stays like this for now.

@MakisH MakisH marked this pull request as ready for review March 20, 2024 08:09
@MakisH MakisH self-assigned this Mar 20, 2024
Copy link
Member

@uekerman uekerman left a comment

Choose a reason for hiding this comment

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

I tried:

  • perpendicular flap: openfoam x {calculix, nutils}
  • oscillator: python x fmi

We still need to add the new files to some cleaning scripts:

  • oscillator/fmi/clean.sh
  • oscillator/python/clean.sh
  • flow-around-controlled-moving-cylinder/controller-fmi/clean.sh
  • flow-around-controlled-moving-cylinder/solid-python/clean.sh
  • ... more cases. Probably all that are not covered by the cleaning tools (elastic-tube-1d, quickstart, oscillator-overlap, more?)

Misc:

  • clean_dumux() is missing clean_case_logs

oscillator/fmi/run.sh Outdated Show resolved Hide resolved
@MakisH
Copy link
Member Author

MakisH commented Mar 20, 2024

I applied all comments (all very good observations!) and switched to a simpler way, which, however, is based on bash.

@uekerman please check again a couple of cases.
@fsimonis please have another look at the general implementation.

@uekerman
Copy link
Member

I again ran oscillator and flow-around-controlled-moving-cylinder and both work nicely.

@MakisH MakisH merged commit 7918d9f into precice:develop Mar 21, 2024
1 check passed
@MakisH MakisH deleted the automatic-logs branch March 21, 2024 09:41
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