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

Logging utilities #51

Merged
merged 52 commits into from
Jul 15, 2024
Merged

Logging utilities #51

merged 52 commits into from
Jul 15, 2024

Conversation

lkdvos
Copy link
Member

@lkdvos lkdvos commented Jul 9, 2024

This PR re-uses MPSKit functionality to handle the logging. I think it is a little cleaner, but I'm definitely open to suggestions.

@lkdvos lkdvos marked this pull request as ready for review July 9, 2024 14:25
Copy link

codecov bot commented Jul 9, 2024

Codecov Report

Attention: Patch coverage is 97.87234% with 1 line in your changes missing coverage. Please review.

Files Coverage Δ
src/PEPSKit.jl 100.00% <ø> (ø)
src/algorithms/peps_opt.jl 95.83% <ø> (ø)
src/algorithms/ctmrg.jl 94.66% <97.87%> (-0.16%) ⬇️

@leburgel
Copy link
Collaborator

Looks good to me, but we should probably increase the verbosity in the examples and tests to at least verbosity=2, since level 1 got less verbose by default. None of the tests actually output log anything CTMRG-related here as far as I can tell. Also, it would be good to document what the different verbosity levels actually do (probably in the CTMRG docstring, which is now out of date with the possible levels).

@pbrehmer
Copy link
Collaborator

I agree, that probably the default verbosity should be changed and that a quick explanation of levels 0 to 3 would be useful to have in the CTMRG docstring. Other than that, I find the new logging machinery really neat (and also makes it more elegant to log on a HPC environment). Thanks!

@lkdvos lkdvos enabled auto-merge (squash) July 14, 2024 18:30
@lkdvos lkdvos disabled auto-merge July 14, 2024 18:30
@lkdvos lkdvos enabled auto-merge (squash) July 14, 2024 18:31
@lkdvos lkdvos requested a review from pbrehmer July 14, 2024 18:47
Copy link
Collaborator

@pbrehmer pbrehmer left a comment

Choose a reason for hiding this comment

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

Except for the verbosity in PEPSOptimize I find this really nice and useful. Thanks!

src/algorithms/peps_opt.jl Outdated Show resolved Hide resolved
@lkdvos lkdvos merged commit 9e53fc8 into master Jul 15, 2024
3 of 8 checks passed
@lkdvos lkdvos deleted the logging branch July 15, 2024 13:00
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.

3 participants