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

Include line diff in numdiff output #1164

Merged
merged 1 commit into from Aug 10, 2016

Conversation

class4kayaker
Copy link
Contributor

When running tests, if Numdiff is used only the differing values and their position in the lines in question are shown in the generated diff files. This results in having to either visually do the comparison, use another diff tool or redoing the diff using "numdiff -V" in order to obtain the context of the variations between the two. The change made is to use "numdiff -V" when generating the DIFF_OUTPUT intended to become *.diff or *.diff.failed in order to also include the originating lines when printing the diff.

Is not making use of this flag an intended design decision, a compatibility limitation, or was the capability overlooked?

@bangerth
Copy link
Contributor

That's one way. I thought I used a different approach in ASPECT, where my idea was to show both the output of numdiff and if there is a difference, also the output of diff for context. But I can't seem to find that code right now. Let's just try it with your code and see if we like it!

@bangerth
Copy link
Contributor

/run-tests

@class4kayaker
Copy link
Contributor Author

If you would prefer the approach of numdiff output followed by diff output if a difference exists, I believe I could modify the current approach to correctly do this. I checked deal.II, and that is the method that it uses, so I could start trying to model the diff script on that. It might still be desirable to use the "-V" flag as it will print the lines which vary above the changed values, so it would be possible to view the immediate context with the numerical diff rather than a line number and position. I can currently do tests against the matrix_nonzeros test and a few others I know I have locally varying behavior for, and give samples of the output.

@bangerth bangerth merged commit cca11c2 into geodynamics:master Aug 10, 2016
@bangerth
Copy link
Contributor

Ah, yes, I did that in deal.II apparently.

Not to worry. Let's just try what we get here and if we don't like it, we can always change it again.

@class4kayaker class4kayaker deleted the adjust_test_diff branch August 10, 2016 03:07
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.

None yet

2 participants