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

Fixing bug in local DVs #138

Merged
merged 2 commits into from
May 3, 2022
Merged

Fixing bug in local DVs #138

merged 2 commits into from
May 3, 2022

Conversation

hajdik
Copy link
Contributor

@hajdik hajdik commented May 3, 2022

Purpose

The changes in #130 to the DV classes introduced an issue that caused my cases and the tutorial wing and airfoil to not run, they exited immediately with SNOPT saying the problem was infeasible. I believe the issue is typos in geoDVLocal and geoDVSpanwiseLocal, which this should fix.

The bigger question is why the MACH and pyGeo tests still passed with this issue and how tests could catch it.

Expected time until merged

A day or two, there are only a few changed lines but someone who has cases that use different types of DVs might want to test with this to make sure everything is working.

Type of change

  • Bugfix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (non-backwards-compatible fix or feature)
  • Code style update (formatting, renaming)
  • Refactoring (no functional changes, no API changes)
  • Documentation update
  • Maintenance update
  • Other (please describe)

Testing

Without this change, running the tutorial scripts in Docker (I tried u20-gcc-ompi-latest) will result in an immediate exit with no apparent errors and a 10 14 code from SNOPT.

Checklist

  • I have run flake8 and black to make sure the code adheres to PEP-8 and is consistently formatted
  • I have run unit and regression tests which pass locally with my changes
  • I have added new tests that prove my fix is effective or that my feature works
  • I have added necessary documentation

@hajdik hajdik requested a review from a team as a code owner May 3, 2022 18:24
@hajdik hajdik requested review from joanibal and anilyil May 3, 2022 18:24
@@ -229,12 +229,6 @@ def __init__(self, name, lower, upper, scale, axis, coefListIn, mask, config, se
super().__init__(name=name, value=np.zeros(nVal, "D"), nVal=nVal, lower=None, upper=None, scale=scale)

self.config = config
if lower is not None:
Copy link
Contributor

Choose a reason for hiding this comment

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

why are you removing these lines?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That portion is duplicated in the class it inherits from, geoDV, so it shouldn't be necessary here.

@codecov
Copy link

codecov bot commented May 3, 2022

Codecov Report

Merging #138 (0ab2d77) into main (2a178fd) will decrease coverage by 10.91%.
The diff coverage is 100.00%.

@@             Coverage Diff             @@
##             main     #138       +/-   ##
===========================================
- Coverage   62.62%   51.70%   -10.92%     
===========================================
  Files          45       45               
  Lines       11219    11213        -6     
===========================================
- Hits         7026     5798     -1228     
- Misses       4193     5415     +1222     
Impacted Files Coverage Δ
pygeo/parameterization/designVars.py 77.04% <100.00%> (+0.32%) ⬆️
pygeo/parameterization/DVGeoMulti.py 0.40% <0.00%> (-89.42%) ⬇️
pygeo/constraints/areaConstraint.py 50.50% <0.00%> (-25.42%) ⬇️
pygeo/constraints/DVCon.py 67.15% <0.00%> (-3.69%) ⬇️
pygeo/pyBlock.py 45.37% <0.00%> (-1.65%) ⬇️
pygeo/parameterization/DVGeo.py 63.91% <0.00%> (-0.50%) ⬇️
pygeo/topology.py 54.92% <0.00%> (-0.23%) ⬇️
pygeo/__init__.py 100.00% <0.00%> (+10.52%) ⬆️
pygeo/parameterization/__init__.py 100.00% <0.00%> (+14.28%) ⬆️

📣 Codecov can now indicate which changes are the most critical in Pull Requests. Learn more

@marcomangano marcomangano merged commit cad9040 into mdolab:main May 3, 2022
@hajdik hajdik deleted the dv-bugfix branch May 3, 2022 19:32
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