-
Notifications
You must be signed in to change notification settings - Fork 113
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
Prettify summary output #333
Conversation
This is the current output:
It would be great to get some feedback on this... |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks much cleaner than before! Nice work overall, @sloede 👍
Another minor comments/suggestion: What about printing |
Codecov Report
@@ Coverage Diff @@
## dev #333 +/- ##
==========================================
- Coverage 88.51% 88.43% -0.09%
==========================================
Files 76 76
Lines 8793 8886 +93
==========================================
+ Hits 7783 7858 +75
- Misses 1010 1028 +18
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
Some additional things coming to my mind:
|
Is this regarding the way it is shown in the summary callback or do you mean we should adapt/implement a |
Isn't that basically the same? I observed how it was printed by the summary callback from the semidiscretization and thought it might be nice to print it using the exported version |
Yes, I fully agree. The first 2 are just missing, I'll see to that. The other two are probably more difficult to achieve. If you have an idea for how to implement it, feel free to let me know. Otherwise, I will - as discussed today - try to think about a clean solution and if it amounts to nothing, postpone it. |
Co-authored-by: Hendrik Ranocha <ranocha@users.noreply.github.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Feel free to merge this and open an issue to improve the output of things like split-form & shock-capturing volume integral types or the AMR controller etc.
With the latest commit (e8ed070), we now get the following: Nested output (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Really nice work, @sloede! Once my comments comments are resolved, please feel free to merge this PR.
Co-authored-by: Hendrik Ranocha <ranocha@users.noreply.github.com>
Currently I see no sane way of including information about a restart, without adding additional fields to an existing data structure, wit the sole purpose of triggering a different output in the summary callback. Do I understand it correctly, that since restarting right now is essentially stateless, we do not have the capability to distinguish a restart from a regular start? |
Yes, you're right. |
Closes #302.