Custom pointsets for dvgeo #299
Conversation
… groups. derivatives not added yet
Codecov ReportAttention: Patch coverage is
❌ Your patch check has failed because the patch coverage (39.65%) is below the target coverage (50.00%). You can increase the patch coverage or adjust the target coverage. Additional details and impacted files@@ Coverage Diff @@
## main #299 +/- ##
==========================================
- Coverage 41.13% 40.99% -0.14%
==========================================
Files 13 13
Lines 4121 4169 +48
==========================================
+ Hits 1695 1709 +14
- Misses 2426 2460 +34 ☔ View full report in Codecov by Sentry. |
adflow/pyADflow.py
Outdated
| @@ -4523,6 +4595,7 @@ def computeJacobianVectorProductFwd( | |||
| # For the geometric xDvDot perturbation we accumulate into the | |||
| # already existing (and possibly nonzero) xsdot and xvdot | |||
| if xDvDot is not None or xSDot is not None: | |||
| # TODO fix | |||
There was a problem hiding this comment.
Yes, it looks like I forgot to make the necessary changes for this to work. Tagging this PR as draft until I can get to it.
|
Leaving this as a draft PR because after #330 is merged, I want to push a small update to this PR that brings similar ptset changes to the changes that come from that other PR. |
|
This should be ready for review again. |
There was a problem hiding this comment.
Hate to be that guy, but is there a test we can add for this?
Also, there are a lot of if else blocks added here that bascially do the same thing, either once for the design family group or once for each custom point set family. Couldn't we avoid all this additional logic if we treated the design family group as a custom point set family?
| if self.customPointSetFamilies is None: | ||
| # we dont have a custom child dvgeo mapping. the surface family will be only | ||
| # designFamilyGroup and we will have a single pointset | ||
| ptSetName = f"adflow_{self.designFamilyGroup}_{aeroProblem.name}_coords" | ||
| ptSetNames[self.designFamilyGroup] = ptSetName | ||
| else: | ||
| # we have a custom surface family to child dvgeo mapping. | ||
| for familyName in self.customPointSetFamilies.keys(): | ||
| ptSetName = f"adflow_{familyName}_{aeroProblem.name}_coords" | ||
| ptSetNames[familyName] = ptSetName |
There was a problem hiding this comment.
Is the use of customPointSetFamilies exclusive to cases where you use child DVGeo's? If not then can you re-word these comments to avoid talking about child dvgeos as it's a bit confusing.
| @@ -3334,11 +3368,52 @@ def setAeroProblem(self, aeroProblem, releaseAdjointMemory=True): | |||
| # we already communicated the points when loading the file, | |||
| # so just add them to dvgeo now | |||
| procPts = surfPts[disp[self.comm.rank] : disp[self.comm.rank + 1]] | |||
| self.DVGeo.addPointSet(procPts, surfPtSetName, **self.pointSetKwargs) | |||
| self.DVGeo.addPointSet(procPts, surfPtSetName, **self.pointSetKwargs, **surfPointSetKwargs) | |||
There was a problem hiding this comment.
How does passing two sets of kwargs to addPointSet work? What's the difference between pointSetKwargs and surfPointSetKwargs?
Purpose
This PR adds the capability to specify custom pointsets based on family names that are tracked separately in DVGeo. Each pointset can receive its own custom kwargs. See the relevant PR in baseclasses for how the custom families are specified: mdolab/baseclasses#89
This is a somewhat complete implementation but there are several assumptions. First, the custom families should not overlap. The implication here comes up with derivative computations; if families overlap, the surface seeds of the overlapped nodes will be double counted.
Second, we assume the custom families span the selected design family, although this is not a hard requirement. We may want to add a check if the provided families actually do span the entire design family.
Expected time until merged
Type of change
Testing
Checklist
flake8andblackto make sure the Python code adheres to PEP-8 and is consistently formattedfprettifyor C/C++ code withclang-formatas applicable