Fix shape handling in `nom_addGlobalDV` #194

A-CGray · 2023-04-04T14:34:27Z

Purpose

This PR fixes the nom_addGlobalDV method in the MPhys wrapper so that it accepts scalar arguments to the value argument, just like the actual addGlobalDV method does. Previously this would fail because nom_addGlobalDV would try and evaluate len(value). The shape of the DV is now automatically extracted from the DV after it is added to the DVGeo object.

Also, I added a docstring because docs are nice.

Expected time until merged

1 week

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

Checklist

I have run flake8 and black to make sure the Python code adheres to PEP-8 and is consistently formatted
I have formatted the Fortran code with fprettify or C/C++ code with clang-format as applicable
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

codecov · 2023-04-04T14:52:53Z

Codecov Report

Merging #194 (5e2827f) into main (0e6b356) will decrease coverage by 0.02%.
The diff coverage is 0.00%.

@@            Coverage Diff             @@
##             main     #194      +/-   ##
==========================================
- Coverage   64.88%   64.87%   -0.02%     
==========================================
  Files          47       47              
  Lines       11939    11941       +2     
==========================================
  Hits         7747     7747              
- Misses       4192     4194       +2

Impacted Files	Coverage Δ
pygeo/mphys/mphys_dvgeo.py	`0.00% <0.00%> (ø)`

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

hajdik · 2023-04-06T19:16:19Z

pygeo/mphys/mphys_dvgeo.py

+        """Add a global design variable to the DVGeo object. This is a wrapper for the DVGeo.addGlobalDV method.
+
+        Parameters
+        ----------
+        dvName : str
+            A unique name to be given to this design variable group
+
+        value : float, or iterable list of floats
+            The starting value(s) for the design variable. This
+            parameter may be a single variable or a numpy array
+            (or list) if the function requires more than one
+            variable. The number of variables is determined by the
+            rank (and if rank ==1, the length) of this parameter.
+
+        func : python function
+            The python function handle that will be used to apply the
+            design variable
+
+        childIdx : int, optional
+            The zero-based index of the child FFD, if this DV is for a child FFD.
+            The index is defined by the order in which you add the child FFD to the parent.
+            For example, the first child FFD has an index of 0, the second an index of 1, and so on.
+
+        isComposite : bool, optional
+            Whether this DV is to be included in the composite DVs, by default False
+
+        Raises
+        ------
+        RuntimeError
+            Raised if the underlying DVGeo object is not an FFD
+        """


Do we want to start putting docstrings in the MPhys wrapper? We'd have to maintain the common attributes in two places then and that could get messy.

At the very very least there should be a docstring that explains the MPhys-specific arguments to the function, for the remainder of the arguments it might be fine to point to the docstring for the original DVGeo function.

bernardopacini

This looks good to me! I agree that we should start adding doc strings for anything that is MPhys-specific, but leave the rest for the pyGeo core.

A-CGray · 2023-04-10T17:40:31Z

I have no idea what is going on with the docs build

WARNING: autodoc: failed to import class 'mphys.mphys_dvgeo.OM_DVGEOCOMP' from module 'pygeo'; the following exception was raised:
Traceback (most recent call last):
  File "/home/docs/checkouts/readthedocs.org/user_builds/mdolab-pygeo/envs/194/lib/python3.7/site-packages/sphinx/ext/autodoc/importer.py", line 58, in import_module
    return importlib.import_module(modname)
  File "/home/docs/checkouts/readthedocs.org/user_builds/mdolab-pygeo/envs/194/lib/python3.7/importlib/__init__.py", line 127, in import_module
    return _bootstrap._gcd_import(name[level:], package, level)
  File "<frozen importlib._bootstrap>", line 1006, in _gcd_import
  File "<frozen importlib._bootstrap>", line 983, in _find_and_load
  File "<frozen importlib._bootstrap>", line 967, in _find_and_load_unlocked
  File "<frozen importlib._bootstrap>", line 677, in _load_unlocked
  File "<frozen importlib._bootstrap_external>", line 728, in exec_module
  File "<frozen importlib._bootstrap>", line 219, in _call_with_frames_removed
  File "/home/docs/checkouts/readthedocs.org/user_builds/mdolab-pygeo/checkouts/194/pygeo/mphys/__init__.py", line 1, in <module>
    from .mphys_dvgeo import OM_DVGEOCOMP
  File "/home/docs/checkouts/readthedocs.org/user_builds/mdolab-pygeo/checkouts/194/pygeo/mphys/mphys_dvgeo.py", line 4, in <module>
    import openmdao.api as om
  File "/home/docs/checkouts/readthedocs.org/user_builds/mdolab-pygeo/envs/194/lib/python3.7/site-packages/openmdao/api.py", line 20, in <module>
    from openmdao.components.exec_comp import ExecComp
  File "/home/docs/checkouts/readthedocs.org/user_builds/mdolab-pygeo/envs/194/lib/python3.7/site-packages/openmdao/components/exec_comp.py", line 1266, in <module>
    if Version(scipy.__version__) >= Version("1.5.0"):
  File "/home/docs/checkouts/readthedocs.org/user_builds/mdolab-pygeo/envs/194/lib/python3.7/site-packages/packaging/version.py", line 195, in __init__
    match = self._regex.search(version)
TypeError: expected string or bytes-like object

eirikurj · 2023-04-17T14:46:31Z

Seems to be missing the mphys module here

pygeo/doc/conf.py

Line 33 in 0e6b356

    
           autodoc_mock_imports = ["numpy", "mpi4py", "scipy", "pyspline", "baseclasses", "pysurf", "prefoil", "pyOCSM", "openvsp"]

eirikurj · 2023-04-17T16:52:45Z

Hmm, actually, reading this again I think its openmdao that is the issue. Let me push something quick.

A-CGray · 2023-04-17T22:13:49Z

Oh nice, thanks for fixing that. This is ready to go now @bernardopacini @hajdik

bernardopacini

LGTM! Thanks @A-CGray

sseraj · 2023-04-21T18:47:50Z

doc/requirements.txt

@@ -1,2 +1,4 @@
 numpydoc
 sphinx_mdolab_theme
+openmdao
+mphys


Are these necessary or is the mock import enough?

bernardopacini

Good catch @sseraj. Looks good to me again.

A-CGray added 2 commits April 4, 2023 10:29

Fix shape handling in nom_addGlobalDV

05c5e1d

cleanup docstring

e5b7aa5

A-CGray requested a review from a team as a code owner April 4, 2023 14:34

A-CGray requested review from hajdik, sseraj, anilyil and bernardopacini and removed request for sseraj April 4, 2023 14:34

hajdik reviewed Apr 6, 2023

View reviewed changes

bernardopacini previously approved these changes Apr 7, 2023

View reviewed changes

A-CGray added 2 commits April 10, 2023 13:34

Point to original DVGeo method in docstring

8910a73

Create mphys component docs page

92a6e56

A-CGray dismissed bernardopacini’s stale review via 92a6e56 April 10, 2023 17:34

Add openmdao to docs requirements

7762396

Update conf.py

2fc70d3

A-CGray added 3 commits April 17, 2023 10:58

Update requirements.txt

9ba1947

Update conf.py

ccf72d0

black -l 120 .

8066abc

minor edit

49995d6

A-CGray requested review from bernardopacini and hajdik April 17, 2023 22:13

hajdik previously approved these changes Apr 20, 2023

View reviewed changes

bernardopacini previously approved these changes Apr 21, 2023

View reviewed changes

sseraj reviewed Apr 21, 2023

View reviewed changes

Update requirements.txt

5e2827f

hajdik dismissed stale reviews from bernardopacini and themself via 5e2827f April 21, 2023 19:05

bernardopacini self-requested a review April 21, 2023 19:08

bernardopacini approved these changes Apr 21, 2023

View reviewed changes

sseraj approved these changes Apr 21, 2023

View reviewed changes

hajdik merged commit f6a44b1 into main Apr 21, 2023

hajdik deleted the allowScalarDVMPhys branch April 21, 2023 19:51

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix shape handling in `nom_addGlobalDV` #194

Fix shape handling in `nom_addGlobalDV` #194

A-CGray commented Apr 4, 2023

codecov bot commented Apr 4, 2023 •

edited

Loading

hajdik Apr 6, 2023

A-CGray Apr 6, 2023

bernardopacini left a comment

A-CGray commented Apr 10, 2023

eirikurj commented Apr 17, 2023

eirikurj commented Apr 17, 2023

A-CGray commented Apr 17, 2023

bernardopacini left a comment

sseraj Apr 21, 2023

bernardopacini left a comment

Fix shape handling in nom_addGlobalDV #194

Fix shape handling in nom_addGlobalDV #194

Conversation

A-CGray commented Apr 4, 2023

Purpose

Expected time until merged

Type of change

Testing

Checklist

codecov bot commented Apr 4, 2023 • edited Loading

Codecov Report

hajdik Apr 6, 2023

Choose a reason for hiding this comment

A-CGray Apr 6, 2023

Choose a reason for hiding this comment

bernardopacini left a comment

Choose a reason for hiding this comment

A-CGray commented Apr 10, 2023

eirikurj commented Apr 17, 2023

eirikurj commented Apr 17, 2023

A-CGray commented Apr 17, 2023

bernardopacini left a comment

Choose a reason for hiding this comment

sseraj Apr 21, 2023

Choose a reason for hiding this comment

bernardopacini left a comment

Choose a reason for hiding this comment

Fix shape handling in `nom_addGlobalDV` #194

Fix shape handling in `nom_addGlobalDV` #194

codecov bot commented Apr 4, 2023 •

edited

Loading