Skip to content

Feature coverage#234

Merged
planes merged 24 commits intodevelopfrom
feature-coverage
Apr 19, 2018
Merged

Feature coverage#234
planes merged 24 commits intodevelopfrom
feature-coverage

Conversation

@WallyMaier
Copy link
Contributor

Just a general code clean up.
Added regression for Ducted Fan and NACA airfoil geometry for coverage.

# rayleigh
# ----------------------------------------------------------------------

<<<<<<< HEAD
Copy link
Member

Choose a reason for hiding this comment

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

Unmerged

Copy link
Contributor Author

Choose a reason for hiding this comment

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

well that's weird

@coveralls
Copy link

coveralls commented Apr 12, 2018

Coverage Status

Coverage increased (+2.8%) to 63.284% when pulling a98e4ef on feature-coverage into c56ba5c on develop.

@WallyMaier WallyMaier closed this Apr 13, 2018
@WallyMaier WallyMaier reopened this Apr 13, 2018
Copy link
Member

@planes planes left a comment

Choose a reason for hiding this comment

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

Make some changes. Talk to Matthew about one issue

@@ -1,167 +0,0 @@
cruise
Copy link
Member

Choose a reason for hiding this comment

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

Why is this AVL script deleted?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Idk...why, but the regression/travis works.... @mclarke2 might be able to comment on the use of it.

actual.takeoff = 99.7
actual.side_line = 97.2
actual.landing = 105.2
actual.takeoff = 99.982372547196633
Copy link
Member

Choose a reason for hiding this comment

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

Hmm, I think these values actually come from Shevell data as a test case. I think what you have done is right. However, we should save the original values in comments somewhere.

@@ -0,0 +1,102 @@
# constnat_temperature.py
Copy link
Member

Choose a reason for hiding this comment

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

Typo

@@ -0,0 +1,73 @@
# NACA_airfoil_compute.py
Copy link
Member

Choose a reason for hiding this comment

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

Name

Cm_a_sur[ii,jj] = cm_alpha_surrogate.predict(np.array([AoA_mesh[ii,jj],mach_mesh[ii,jj]]))
Cn_b_sur[ii,jj] = cn_beta_surrogate.predict(np.array([AoA_mesh[ii,jj],mach_mesh[ii,jj]]))
NP_sur[ii,jj] = neutral_point_surrogate.predict(np.array([AoA_mesh[ii,jj],mach_mesh[ii,jj]]))
NP_sur[ii,jj] = neutral_point_surrogate.predict(np.array([AoA_mesh[ii,jj],mach_mesh[ii,jj]]))
Copy link
Member

Choose a reason for hiding this comment

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

@mclarke2 you need to approve this change.

Copy link
Contributor Author

@WallyMaier WallyMaier Apr 18, 2018

Choose a reason for hiding this comment

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

There wasn't a change...The commented code below allows the sklearn to work on my own machine. I assume my version is more up to date than the SUAVE repo.

Copy link
Member

Choose a reason for hiding this comment

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

Keep those differences locally...have @mclarke2 comment on it

g = self.compute_gamma(T,p)
else:
g = 1.4*np.ones_like(T)

Copy link
Member

Choose a reason for hiding this comment

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

Is this a necessary change?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

nope, I changed It...


T = np.resize(T,[n_a,3,3])

return T
Copy link
Member

Choose a reason for hiding this comment

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

You should make this a regression

Copy link
Contributor Author

Choose a reason for hiding this comment

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

So this is already being covered...are you sure you want another regression?

Copy link
Member

Choose a reason for hiding this comment

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

No leave it alone

@planes planes merged commit ba05f17 into develop Apr 19, 2018
@WallyMaier WallyMaier deleted the feature-coverage branch September 24, 2018 16:39
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