Clean jastrow output #1525
Merged
Clean jastrow output #1525
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The builder, and the bspline and polynomial 3-body factors. Put the parameters in the debug output.
Summary of number of parameters for each type of correlation.
ghost
assigned 
|
It would be good to also print out the value of the "function" XML attribute, i.e. "bspline" in the case of J1/J2 and "polynomial" for J3. |
|
I think it would be consistent with our atomic position output to also print the Jastrow parameters with verbosity=high. I am happy with not printing them by default. |
|
Q. What is printed now in an optimization run? Does the optimizer trigger printing of the parameters? |
|
@markdewing can we have more compact print out? @prckent our |
|
I also don't see a reason not printing cusp and cutoff by default. My feeling is any effective control parameters given by user from xml must be always reflected in the output. Users should not need debug if they need to verify their input. |
|
@ye-luo Make an issue or PR to adjust verbosity=high. Certainly there are is a long term issue about what to do with large numbers of parameters, but that is not something to be solved in this PR. |
|
General comment on these output improvements: the main goal is to get to sensible, logically formatted output that is interpretable to a general user. I don't have strong opinions on what is printed by default vs high. For sure we each have some biases that reflect the calculations we do. We can adjust what is/is not printed at each output level over time. At the moment there is XML, redundancy, variables of unclear meaning, printing of internal code status etc., so cleanup is badly needed. Debug can be ugly but the default output should be readily understandable. |
|
@prckent issue created. |
|
A bit more complicated than I thought. May be it is time to rename put() to parseXML() and also add an output stream as an argument to have a better print control of all the functors. |
Add the function attribute to the output. Add parameter output at high verbosity. Cusp and cutoff are printed at low verbosity. No space before colons, to be consistent with previous output cleanup.
|
Output for low verbosity: Output for high verbosity: |
ye-luo
approved these changes
Apr 3, 2019
prckent
approved these changes
Apr 3, 2019
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This organizes some of the output for the Jastrow builder, and for the bspline and the polynomial 3-body Jastrows (which seem to the be most commonly used ones).
The output of parameter values is relegated to the debug output.
The new output for high verbosity:
The output for low verbosity: