Various fixes by talbring · Pull Request #798 · su2code/SU2

talbring · 2019-10-10T13:36:06Z

Proposed Changes

This PR fixes a couple of bugs I found recently. I also adapted the config preprocessing output to reflect the new output and convergence criteria options.

Furthermore it fixes the artifacts mentioned by @TobiKattmann in #774 regarding the Heatflux sensitivity.

Feel free to add any other fixes that you might came across.

Related Work

PR Checklist

Put an X by all that apply. You can fill this out after submitting the PR. If you have any questions, don't hesitate to ask! We want to help. These are a guide for you to know what the reviewers will be looking for in your contribution.

I am submitting my contribution to the develop branch.
My contribution generates no new compiler warnings (try with the '-Wall -Wextra -Wno-unused-parameter -Wno-empty-body' compiler flags).
My contribution is commented and consistent with SU2 style.
[] I have added a test case that demonstrates my contribution, if necessary.

…_fixes

…ixes

…y is requested.

TobiKattmann · 2019-10-11T22:05:36Z

@oleburghardt @talbring This PR fixes the multicore heat-flux-sensitivitiy issues. Tested with the tutorial made by ole. Good job

I added a commit that fixes the error-message if you want to have tecplot-binary output but compiled using the --disable-tecio flag. It caused a compile time error ... it is behind a preprocessor statement #ifndef HAVE_TECIO, so this thing is only seen when using the --disable-tecio flag.

…ixes

… computation.

…ixes

pcarruscag · 2019-10-15T10:25:07Z

Common/include/toolboxes/C2DContainer.hpp

+   * \note Aligned dynamic allocation is disabled for compilation on MAC.
   */
-#define REAL_ALLOCATOR(EXTRA)                                           \
+#ifndef __APPLE__


Addresses the problem reported by @economon in #753

This now build on Mac, but I now see issues with the assert() calls on the first access of one of these containers at the start of the solver

@economon I need more details, I do not have a Mac to go test this myself.

Here's what I am seeing in Xcode (trace on left and assertion failure at bottom). Just let me know what else I can provide to help with this...

Humm, is this after you updated #790? (I think I recognise the function name) If so that probably means the data is not being allocated. Push your changes and I can have a look directly in #790.

Do note that asserts are disabled when building with ninja unless buildtype=debug.
You may want to define NDEBUG for your optimized builds in Xcode as the cost of those checks is not negligible.

Noted. I typically only build in serial debug mode in Xcode and keep a separate build at the command line in release mode w/ MPI, but I can add flags for this.

Btw, I also noticed that the new C2DContainer class has the assignment operator overloaded to perform a deep copy. Is it possible to do a shallow copy, i.e., just set the pointer of an existing initialized container to an existing one without allocating memory?

With what is implemented it is not possible. If the objective is avoiding the double allocation of gradients, I would suggest using a VectorOfMatrix& that is set to primitive gradient when no allocation is needed, otherwise a different VectorOfMatrix object is resized and the the reference is set to that instead. This way there are no ownership problems (who frees the pointer and so on). References need an initializer but that can be either of the other containers.
If it's for a different purpose I can think of something.

Aok, let me give that a try (yes it is for the gradients). #790 is looking good after merging everything, will be pushing the cleaned version shortly

Looks like the choice of initializer does affect the solution, btw. Looks like the reconstruction gradient vector should use the auxiliary vector's initializer for correctness...

pcarruscag · 2019-10-15T10:28:05Z

Common/src/linear_algebra/CSysMatrix.cpp

    ILU_matrix = new ScalarType [nnz_ilu*nVar*nEqn];
    for (iVar = 0; iVar < nnz_ilu*nVar*nEqn; iVar++) ILU_matrix[iVar] = 0.0;

+    invM = new ScalarType [nPointDomain*nVar*nEqn];


Addresses segfault found by @economon when using ILU with fill-in greater than 0.
Maybe in #790 one of the testcases can be modified to use this feature?

economon · 2019-10-15T20:49:03Z

init.py

+    cur_sha_commit = status[1:].split(' ')[0]
+    if (cur_sha_commit != sha_commit):
+      print('SHA-1 tag stored in index does not match SHA tag stored in this script.')
+      sys.exit(1)


Has the index been updated to have these new SHA tags for meson and ninja? I am still debugging #790 upon merging #753 #777 and this #798, and now when I try to build with meson, I get this error. Could be that my local copy still has some issues due to improper merge, or could be that this isn't updated yet

Previously the tags in git and in the init.py script where not matching and the error was not thrown because the indentation was wrong ... try git submodule update or deleting meson, ninja, codi, medi in the externals folder.

I continued to have this issue, so I simply reset the stored SHA in the index to the one that you have in init.py. Should be all good. Will be adjusted in #790

It seems like you changed the version of ninja and meson in your branch. Maybe just make sure to use the newest ones if they work without any problem.

talbring · 2019-10-22T09:25:24Z

I tried to solve the problem where sometimes the regression tests stall ... it seems to be a problem of openmpi. Changed it to mpich, but then mpi4py has problems. I reverted it for now. We have to live with that problem for a while unfortunately.

…Nonmatching Set* and Add* name.

…or of the former

pcarruscag · 2019-10-23T16:53:45Z

Common/src/config_structure.cpp

  /* DESCRIPTION: Number of nonlinear deformation iterations (surface deformation increments) */
  addUnsignedLongOption("DEFORM_NONLINEAR_ITER", GridDef_Nonlinear_Iter, 1);
-  /* DESCRIPTION: Number of smoothing iterations for FEA mesh deformation */
-  addUnsignedLongOption("DEFORM_LINEAR_ITER", GridDef_Linear_Iter, 1000);


I got rid of this option in favor of the equivalent and more descriptive DEFORM_LINEAR_SOLVER_ITER

…ete adjoint (they are more instructional now). Reducing code lines for variable registration by (default) function arguments.

…ge mistake?).

…arious_fixes

oleburghardt · 2019-10-25T14:27:01Z

@pcarruscag
I added a commit that improves the bad names for the new routines that we discussed #803. They should be more instructional now.
Moreover, at some places (default) function arguments are used instead of different functions, which hides the changes a bit and makes it more elegant.

TobiKattmann

Hey all, is this still WIP? One could merge this and reopen another various_fixes branch. I find myself and also e.g. #790 tracking this branch rather than develop as it includes fixes I need (which unnecessarily increases diff size to develop for said PR790).

Tests pass and (in serial) are no additional warnings, so from my standpoint this would be good to go

I will address the 2 little points below

TobiKattmann · 2019-10-27T17:46:14Z

Common/src/interpolation_structure.cpp

-            if (dist == 0.0) break;
          }
-
+          


trailing whitespace Edit: deleted

TobiKattmann · 2019-10-27T17:48:02Z

Common/src/interpolation_structure.cpp

@@ -1379,13 +1383,13 @@ void CMirror::Set_TransferCoeff(CConfig **config) {
    Buffer_Send_FaceIndex   = new unsigned long[MaxFace_Donor];
    Buffer_Send_FaceNodes   = new unsigned long[MaxFaceNodes_Donor];
    //Buffer_Send_FaceProc    = new unsigned long[MaxFaceNodes_Donor];


Can this line be erased (and 5 lines below again)? Edit: I erased them

TobiKattmann · 2019-10-27T18:01:21Z

Common/src/interpolation_structure.cpp

+            if (Buffer_Receive_GlobalPoint[Global_Point_Donor] != -1){
+
+              Coord_j = &Buffer_Receive_Coord[ Global_Point_Donor*nDim];
+
+              dist = PointsDistance(Coord_i, Coord_j);
+
+              if (dist < mindist) {
+                mindist = dist; pProcessor = iProcessor; 
+                pGlobalPoint = Buffer_Receive_GlobalPoint[Global_Point_Donor];                
+              }


Thanks again @talbring @rsanfer (and everyone involved I might not know about) for finding & fixing this bug!

talbring · 2019-10-27T19:01:21Z

Yep from my side we can merge this in

oleburghardt · 2019-10-28T07:13:06Z

@TobiKattmann Good idea to eventually just open up another one.

pcarruscag · 2019-10-28T10:47:32Z

And... we broke travis again.
Edit: Ah Tim fixed it already.

talbring added 7 commits October 8, 2019 13:30

Fixing config preprocessing output

423003d

Added message after finishing meson configuration

51741ed

Added message after finishing meson configuration

4af087b

Fixing FFD box output

cfc8984

Fixing sha tag check in init.py

61bc0ec

fixing heat flux computation

492c800

Merge branch 'develop' of github.com:su2code/SU2 into various_fixes

b3028e5

oleburghardt mentioned this pull request Oct 11, 2019

New Discrete Adjoint Multizone Capabilities (e.g. Adjoint CHT) #774

Merged

4 tasks

oleburghardt and others added 8 commits October 11, 2019 11:56

Delete duplicated solver call in discrete adjoint fluid iteration.

9577134

Merge branch 'develop' of https://github.com/su2code/SU2 into various…

dd1c390

…_fixes

Bugfix conjugate heat interface.

f1c4ecf

Move history output inside inner loop of disc adj multizone driver.

38717ea

Merge branch 'various_fixes' of github.com:su2code/SU2 into various_f…

d5dd87b

…ixes

Fixing multizone output

0cdebc6

Adapting test case values after heatflux fix

c5a6a8c

Change error msg if SU2 was compiled without tecio, but tecplot binar…

bf2f6ef

…y is requested.

TobiKattmann mentioned this pull request Oct 11, 2019

New Nonlinear Iteration Controller for Improved Fluid Solver Robustness & Convergence #790

Merged

4 tasks

talbring added 2 commits October 14, 2019 10:45

📝 Add @talbring as a contributor

06de3d9

Merge branch 'various_fixes' of github.com:su2code/SU2 into various_f…

b6c11d7

…ixes

pcarruscag mentioned this pull request Oct 14, 2019

Restructuring of CVariable into a Contiguous Data Structure #753

Merged

4 tasks

talbring and others added 7 commits October 14, 2019 13:55

Fixing tecio error

64cfef9

Reverting contributor stuff for now

79c6cc6

Set old adjoint solution for the multiphysics case to fix (inner) RMS…

c9bc080

… computation.

Merge branch 'various_fixes' of github.com:su2code/SU2 into various_f…

9a84680

…ixes

Fix Automake makefile.

7862624

disable aligned allocation on MAC

cd3e7a2

fix bug in allocation of inverse diagonal for ILU(k!=0)

af52ffd

pcarruscag reviewed Oct 15, 2019

View reviewed changes

economon reviewed Oct 15, 2019

View reviewed changes

talbring added 3 commits October 22, 2019 11:02

Update .travis.yml

403ff3a

Update .travis.yml

85009bd

Reverting travis yml

9335963

TobiKattmann and others added 2 commits October 22, 2019 16:09

Little typo in output for Disc.Adj. BGS_ADJ[OINT]_DISSIPATION fixed. …

4b14803

…Nonmatching Set* and Add* name.

merge options DEFORM_LINEAR_SOLVER_ITER and DEFORM_LINEAR_ITER in fav…

c7c40da

…or of the former

pcarruscag reviewed Oct 23, 2019

View reviewed changes

pcarruscag and others added 7 commits October 23, 2019 19:28

Merge branch 'develop' into various_fixes

9b2b221

remove duplicate option in cfg, change tutorials branch in travis

010f135

fix compiler warnings in interpolation_structure.cpp

fd92e1a

Add nonphysical point output field

1cbe242

Improving names for new variables and routines for multiphysics discr…

5f132b8

…ete adjoint (they are more instructional now). Reducing code lines for variable registration by (default) function arguments.

Removing unnecessary if-case for OldSolution in discrete adjoint (mer…

cfac92f

…ge mistake?).

Merge branch 'various_fixes' of https://github.com/su2code/SU2 into v…

fbdda81

…arious_fixes

TobiKattmann approved these changes Oct 27, 2019

View reviewed changes

Erase a trailing whitespace and remove few commented code lines.

ef13abb

talbring changed the title ~~[WIP] Various fixes~~ Various fixes Oct 27, 2019

pr-triage bot added the PR: unreviewed label Oct 27, 2019

oleburghardt merged commit ac104f8 into develop Oct 28, 2019

oleburghardt deleted the various_fixes branch October 28, 2019 07:13

pr-triage bot added PR: merged and removed PR: unreviewed labels Oct 28, 2019

oleburghardt mentioned this pull request Oct 29, 2019

Various fixes II #807

Merged

4 tasks

talbring added changelog:fix changelog:none and removed changelog:fix labels Nov 6, 2019

Conversation

talbring commented Oct 10, 2019

Proposed Changes

Related Work

PR Checklist

Uh oh!

TobiKattmann commented Oct 11, 2019

Uh oh!

pcarruscag Oct 15, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

talbring commented Oct 22, 2019

Uh oh!

Choose a reason for hiding this comment

Uh oh!

oleburghardt commented Oct 25, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

TobiKattmann left a comment

Choose a reason for hiding this comment

Uh oh!

TobiKattmann Oct 27, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

TobiKattmann Oct 27, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

talbring commented Oct 27, 2019

Uh oh!

oleburghardt commented Oct 28, 2019

Uh oh!

pcarruscag commented Oct 28, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

pcarruscag Oct 15, 2019 •

edited

Loading

oleburghardt commented Oct 25, 2019 •

edited

Loading

TobiKattmann Oct 27, 2019 •

edited

Loading

TobiKattmann Oct 27, 2019 •

edited

Loading

pcarruscag commented Oct 28, 2019 •

edited

Loading