Skip to content

Conversation

@thomasdick
Copy link
Contributor

@thomasdick thomasdick commented Nov 12, 2021

The Coupled RHT-CFD Adjoint did not compare the computed gradient file to the reference file. Instead, the test case passed without file comparison.

Proposed Changes

Added a call to run_filediff to ensure the test is run.
Moved the Coupled RHT-CFD Adjoint test in serial_regression_AD.py down to the file difference test.

Related Work

This fixes the related issue #1431
The reference gradient file in the test case repo needs a small modification. See su2code/TestCases/pull/84

PR Checklist

Put an X by all that apply. You can fill this out after submitting the PR. If you have any questions, don't hesitate to ask! We want to help. These are a guide for you to know what the reviewers will be looking for in your contribution.

  • I am submitting my contribution to the develop branch.
  • My contribution generates no new compiler warnings (try with the '-Wall -Wextra -Wno-unused-parameter -Wno-empty-body' compiler flags, or simply --warnlevel=2 when using meson).
  • My contribution is commented and consistent with SU2 style.
  • I have added a test case that demonstrates my contribution, if necessary.
  • I have updated appropriate documentation (Tutorials, Docs Page, config_template.cpp) , if necessary.

…Coupled RHT-CFD Adjoint test in serial_regression_AD.py.
Copy link
Member

@pcarruscag pcarruscag left a comment

Choose a reason for hiding this comment

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

Thanks for fixing this 👍

Copy link
Contributor

@TobiKattmann TobiKattmann left a comment

Choose a reason for hiding this comment

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

💐 thanks Thomas.
As far as I see, there is a discrepancy for one DV only in that very Testcase, and that is in the last displayed digit (-0.00432385 vs -0.00432384) which I think is ok to just change. So please go ahead and change that. Once serial_regression_AD passes this can be merged from my perspective

@thomasdick
Copy link
Contributor Author

Yes there is a discrepancy in the last digit for one the gradient values.
Since the reference file for the gradient values is in the Testcases repository and not the SU2 repository, I created a small pull request there to change the value. See su2code/TestCases/pull/84.
Together the changes lead to the test passing as expected.

@pcarruscag pcarruscag merged commit 1a7b4e6 into su2code:develop Nov 13, 2021
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.

3 participants