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
Fixed multiple BC DV and setRotationRate rate for local blocks #323
Conversation
Codecov Report
@@ Coverage Diff @@
## main #323 +/- ##
==========================================
+ Coverage 41.94% 41.96% +0.01%
==========================================
Files 13 13
Lines 4005 4006 +1
==========================================
+ Hits 1680 1681 +1
Misses 2325 2325
📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A few preliminary things:
- Please run black on the python layer and fprettify on the fortran layer.
- For the BC fixes, can you add tests that demonstrate your changes fix the bug? Testing on a local case won't be enough here because you are changing manually differentated routines.
- Can you add a test for the rotation rate fix? I would like to see a test that demonstrates the expected capability and shows your changes achieve that capability.
@@ -6309,6 +6310,7 @@ def _getObjectivesAndDVs(self): | |||
"mavgptot": self.adflow.constants.costfuncmavgptot, | |||
"aavgptot": self.adflow.constants.costfuncaavgptot, | |||
"aavgps": self.adflow.constants.costfuncaavgps, | |||
"mavgrho": self.adflow.constants.costfuncmavgrho, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is this cost function that you're adding?
I would prefer two separate PRs. |
Closing this PR due to inactivity. I will shortly make a PR to address the multi BC issue and link it to this PR. There is an issue open (#353 ) that addresses the other aspect of the PR. |
Purpose
Currently when multiple BC DVs of the same type are applied (e.g. bcinflowsubsonic, bcoutflowsubsonic), the second instance of the BC and its derivatives are overwritten on to the first BC and the values of the second BC remain unchanged and its derivatives are zero. The current PR fixes both the BC value change and its derivative update, which are manually differentiated routines.
Fixes
setRotationRate(rotCenter, rotRate, cgnsBlocks=None)
. Currently when cgnsBlocks are passed, adflow adds rotation rate to 'n' cgnsBlocks starting from the first block number instead of applying the rotation rate to the specified block ids. The changes in this PR fix this bug.Expected time until merged
One week
Type of change
Testing
Proper test case to be created.
Checklist