Skip to content

Conversation

@emstoudenmire
Copy link
Contributor

This PR fixes an issue where the outputlevel keyword to the dmrg function does not produce any output. (This was noticed when calling it from the TNS solver code, just for some background context.) The changes include:

  • setting region_printer and sweep_printer to use the definitions in defaults.jl for both interfaces to alternating_update
  • including the inserter_kwargs into the all_kwargs named tuple in region_update so that parameters like cutoff will be available to the printers
  • making sure default_region_printer has defaults (of nothing) for the values cutoff, maxdim, and mindim to make that function robust (i.e. not to assume all of these will be passed, since the current pattern was crashing in cases where only, say, cutoff was passed into the inserter_kwargs)

@emstoudenmire
Copy link
Contributor Author

I tested the current version and it works well. I would have liked to make a unit test, and while I would know how for a custom printer, here it's the default printer so I couldn't think of a good way except for a kind of test that explicitly checks the stdout output.

@mtfishman
Copy link
Member

mtfishman commented Sep 26, 2024

Yeah, testing printing is tricky, since we don't want them to be too specific or else they are constantly breaking, and then we need to update tests any time we change how something prints.

Maybe you could add a test that checks that at least a certain number of lines are printed by default, and that the output contains certain strings we expect, like "cutoff", "maxdim", and "mindim"? That should be pretty robust and we shouldn't have to update it very often.

@emstoudenmire
Copy link
Contributor Author

The tests were working yesterday, except for the documentation, but now are failing.
Interestingly, I can reproduce the testing failure on my laptop, but only when I do activate . and > test within the REPL.
If I run the tests just by running runtests.jl things go better. So it's a bit unclear to me right now what's going on there.

One possible issue, though it may be a red herring, is that on my machine I can't install a testing dependency, "OrdinaryDiffEq" because of a version compatibility issue. (The Pkg error message is quite long and I don't know how to read it, unfortunately.)

@mtfishman
Copy link
Member

I remember seeing OrdinaryDiffEq cause subtle compatibility issues that I wasn't able to understand. I'm trying to run the tests again.

@mtfishman
Copy link
Member

mtfishman commented Sep 26, 2024

@emstoudenmire the Windows tests passed, maybe the tests are just timing out or failing for some other non-Julia-specific reason.

@emstoudenmire
Copy link
Contributor Author

I'm glad those passed. So then we can be pretty sure the code is ok. (The relevant tests were passing locally for me too, I was just having issues running the entire testing suite for some reason.)

Ready to merge?

@mtfishman mtfishman merged commit fc6fac2 into main Sep 26, 2024
3 of 6 checks passed
@mtfishman mtfishman deleted the fix_printer_defaults branch September 26, 2024 21:19
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