Delete dead-code for "nearfield" and "interface" boundaries#1351
Delete dead-code for "nearfield" and "interface" boundaries#1351pcarruscag merged 6 commits intodevelopfrom
Conversation
| if (IndexNF_inv[AngleInt+1] > 0) { iColumn = IndexNF_inv[AngleInt+1]; goto end; } | ||
| if (IndexNF_inv[AngleInt-1] > 0) { iColumn = IndexNF_inv[AngleInt-1]; goto end; } |
There was a problem hiding this comment.
I was almost surprised 😄
There was a problem hiding this comment.
lol, that code looks like someone really hated else if
|
This pull request fixes 6 alerts when merging 02c1e4d into 8e7bd69 - view on LGTM.com fixed alerts:
|
|
@snow54 I think the solution file for your testcase will have to be updated, can you have a look please to make sure the discrete adjoint still works? |
TobiKattmann
left a comment
There was a problem hiding this comment.
Well, you had me at +23 −1,962 ... 💐 I do not have much to say other than thanks for this cleanup (^~^)/
Once yuki confirms that everything works this is good to go from my side
... And 'now' you complain about complex code! Get your things together CodeFactor. (*proceeds to hang off employee of the month Feb2020 picture)
|
@pcarruscag Thanks for the cleanup. I have made a pull request to update solution file for the test case. You have deleted codes that create WeightNF.dat so I have deleted it from python codes as well. (I tried to push as a suggestion but it didn't work as I expected).
I thought so when I was trying to make it work in ver.6 but, when I tried with mesh with two boundaries with the same points, it didn't work. |
|
This pull request fixes 6 alerts when merging d1bae40 into 8e7bd69 - view on LGTM.com fixed alerts:
|
Co-authored-by: TobiKattmann <31306376+TobiKattmann@users.noreply.github.com>
|
This pull request fixes 6 alerts when merging c17195f into 8e7bd69 - view on LGTM.com fixed alerts:
|
|
Thank you for updating the solution file @snow54 , there's quite a big difference in the adjoint residuals, do the final derivatives still make sense? Is the flow field noticeably different (for the better, e.g. smoother? or equivalent to not having the boundary at all?) now that nearfield is treated as an internal boundary? I'm sorry that you had to spend time fixing that MPI code... But at least we found out we could clean all this obsolete code. |
I think we can update the residuals. Gradients between adjoint and finite difference match quite well as shown below. I haven't finished calculating for all design variables, but I think it is enough.
In terms of flow field, the capture below is from the current develop branch. The boundary between structured grid and unstructured gird is a nearfield boundary. An object is above this capture and pressure wave propagates from there. Since this grid is inclined by Mach angle, pressure distribution on the nearfield should be fairly similar to the region above but this capture shows some strange pressure disturbance. Limiter: VAN_ALBADA_EDGE Limiter: VENKATAKRISHNAN_WANG The capture below is from the branch with this PR. The issue I mentioned above does not exist. Limiter: VAN_ALBADA_EDGE I think the residuals for direct solver will be different like the adjoint if you run it for some hundreds more iterations (currently, the test case has only 20 iterations). However, since the nearfield boundary is a bit far from an object, it takes some iterations for pressure waves to reach the nearfield boundary. This PR also solves an issue with VENKATAKRISHNAN_WANG limiter. It seems to be much easier for convergence than VAN_ALBADA_EDGE, so it is fairly useful. I still have a gradient un-match issue with my bigger mesh but I believe it is coming from something else.
Yeah, I noticed it had been deleted. It's OK. It was still a good opportunity for me to learn how MPI works. |
|
This pull request fixes 6 alerts when merging 1243d16 into 1a84301 - view on LGTM.com fixed alerts:
|
I think nearfield was a kind of internal interface between two boundaries with the same points, so not a "do nothing" group of points but more like an actuator disk without any jump.
Anyway, it was all incomplete or broken and there was no documentation in the code, for a change...
To anyone in the future the commit to revert is 265bfe8814ed345e9ab6288265e80858b00ae59e