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

r-test: update of lin file comparison #1694

Merged
merged 5 commits into from
Jul 19, 2023
Merged

Conversation

ebranlard
Copy link
Contributor

@ebranlard ebranlard commented Jul 19, 2023

This pull request is ready to be merged

Feature or improvement description
Comparison of .lin files has been broken so the r-tests have not been working as intended, and were not performing any comparison. I'm not sure since when this is the case.

Instead of using and ad-hoc script for reading the .lin file, this pull request uses a file from the python toolbox to read the lin files (not my greatest script, but it should be fairly robust). For now, I've copy pasted the file in the lib folder. In the future, we could simply git clone the python toolbox.

Errors will be thrown if:

  • the files don't have the same number of lines
  • the files don't have the same variables (vectors and matrices)
  • the first 10 frequencies and damping differ
  • the variables (vectors and matrices) have different dimensions
  • the variables contain NaN or **** values.
  • the operating points, state matrices, and Jacobian are not within a given tolerance.

The script returns fairly verbose messages (which file, variable and array index fails).
For instance:

46: Error:  Case: StC_test_OC4Semi_Linear_Nac: Failed to compare variable `A`, Element [4,7], new : 8.618, baseline: 8.602
46:     in linfile: C:/W0/Work/2018-NREL/BAR-Cone-BEM/openfast-rc3.5.1/build-double-release/reg_tests/glue-codes/openfast\StC_test_OC4Semi_Linear_Nac\StC_test_OC4Semi_Linear_Nac.1.lin.
46:     Exception:
46: Not equal to tolerance rtol=1e-05, atol=1e-05
46:
46: Mismatched elements: 1 / 1 (100%)
46: Max absolute difference: 0.016
46: Max relative difference: 0.00186003
46:  x: array(8.618)
46:  y: array(8.602)

Related issue, if one exists
#1693

Impacted areas of the software
r-test linearization tests.

Additional supporting information
The script currently uses numpy.all_close, but I believe it is the same as the function isclose which was previously implemented. The tolerances atol=1e-5 and rtol=1e-5 are still used.

It might be that if this test is finicky, we will have to revise the comparison of elements.

Test results, if applicable
Currently with atol=1e-5 and rtol=1e-5 the following 4 tests fail:
43 - 5MW_Land_BD_Linear (Failed)
44 - 5MW_OC4Semi_Linear (Failed)
45 - StC_test_OC4Semi_Linear_Nac (Failed)
46 - StC_test_OC4Semi_Linear_Tow (Failed)

The differences are sufficiently large that I don't think that increasing the tolerance value will make the test pass. We'll likely have to redo the baseline.

Checklist

  • Update the baselines

@andrew-platt
Copy link
Collaborator

This has definitely been needed.

One future addition that would be useful is to calculate the eigenvalues to get the frequencies. Those are almost more important than the matrices themselves.

@ebranlard
Copy link
Contributor Author

I've now tuned a bit the rtol/atol for the matrix comparisons based on the reasoning that we only have 3 digits after the decimal, so we can expect a rtol between 1e-3 and 1e-4. I've used 2e-3 to give some room for numerical errors (e.g. the thrid decimal point chaning a tiny bit).

I've also added test for the frequencies and damping. These tests occur before the matrix/states comparisons.
These tests might need tuning in the future.

With the tuning in rtol atal, only those two tests fails:
44 - 5MW_Land_BD_Linear (Failed)
45 - 5MW_OC4Semi_Linear (Failed)

The frequency/damping test indicate some changes in damping that occured.

44:  Case: 5MW_Land_BD_Linear: freq_ref: [0.72878 1.12103 2.09079 3.78    4.95992 5.65912 8.45789 9.93836] [Hz]
44:  Case: 5MW_Land_BD_Linear: freq_new: [0.72931 1.1223  2.09171 3.77941 4.96181 5.65907 8.45812 9.93833] [Hz]
44:  Case: 5MW_Land_BD_Linear: damp_ref: [0.21179 0.33626 0.62722 1.17288 1.53229 1.75916 2.6449  3.09872] [%]
44:  Case: 5MW_Land_BD_Linear: damp_new: [0.44912 0.46434 1.31406 1.58554 3.09832 3.86349 3.34208 6.81007] [%]
44:
44:
44: Error:  Case: 5MW_Land_BD_Linear: Failed to compare A-matrix damping ratios
44:     Linfile: [..]/build-double-release/reg_tests/glue-codes/openfast\5MW_Land_BD_Linear\5MW_Land_BD_Linear.1.BD1.lin.
44:     Exception:
44:     Not equal to tolerance rtol=0.01, atol=0.1
44:
44:     Mismatched elements: 9 / 10 (90%)
44:     Max absolute difference: 3.7113488
44:     Max relative difference: 1.19770363
44:      x: array([0.449119, 0.464341, 1.314057, 1.58554 , 3.098321, 3.863494,
44:            3.342082, 6.810069, 5.997717, 4.684149])
44:      y: array([0.211794, 0.336262, 0.627221, 1.172881, 1.532291, 1.759163,
44:            2.644899, 3.098721, 3.635833, 4.684131])
45:  Case: 5MW_OC4Semi_Linear: freq_ref: [0.00887 0.00887 0.01262 0.03147 0.03892 0.03911 0.04956 0.05786] [Hz]
45:  Case: 5MW_OC4Semi_Linear: freq_new: [0.00887 0.00887 0.01337 0.03147 0.0468  0.04719 0.04956 0.05786] [Hz]
45:  Case: 5MW_OC4Semi_Linear: damp_ref: [ 0.99833  0.99669  0.5148  56.73884  1.12806  1.12051 64.05133  1.13575] [%]
45:  Case: 5MW_OC4Semi_Linear: damp_new: [ 0.99824  0.9968   0.66051 56.73884  0.24683  0.93971 64.05133  1.13575] [%]
45:
45:
45: Error:  Case: 5MW_OC4Semi_Linear: Failed to compare A-matrix damping ratios
45:     Linfile: [..]/reg_tests/glue-codes/openfast\5MW_OC4Semi_Linear\5MW_OC4Semi_Linear.1.lin.
45:     Exception:
45:     Not equal to tolerance rtol=0.01, atol=0.1
45:
45:     Mismatched elements: 3 / 10 (30%)
45:     Max absolute difference: 0.88122953
45:     Max relative difference: 0.78118733
45:      x: array([ 0.998244,  0.996797,  0.660512, 56.738842,  0.246835,  0.939708,
45:            64.051334,  1.135753, 23.225509, 11.672431])
45:      y: array([ 0.998335,  0.996689,  0.514802, 56.738842,  1.128064,  1.120507,
45:            64.051334,  1.135754, 23.225509, 11.672431])

@ebranlard ebranlard added this to the v3.5.1 milestone Jul 19, 2023
@ebranlard ebranlard merged commit 4a4423a into OpenFAST:rc-3.5.1 Jul 19, 2023
@ebranlard ebranlard deleted the f/lincomp branch July 19, 2023 19:59
@andrew-platt andrew-platt mentioned this pull request Oct 19, 2023
19 tasks
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.

2 participants