example/step-7: Update indenting and modernize #6781

tamiko · 2018-06-15T23:57:27Z

In reference to #6774

tamiko · 2018-06-15T23:58:24Z

examples/step-7/step-7.cc

-    static const double       width;
+    static const unsigned int                             n_source_centers = 3;
+    static const std::array<Point<dim>, n_source_centers> source_centers;
+    static const double                                   width;


Question: Shall we switch to std::array here?

I am in favor, but we have to explain why we can. In particular, we should use constexpr then.

tamiko · 2018-06-15T23:59:26Z

examples/step-7/step-7.cc

+                  const auto center = cell->face(face_number)->center();
+                  if ((std::fabs(center(0) - (-1)) < 1e-12) ||
+                      (std::fabs(center(1) - (-1)) < 1e-12))
+                    cell->face(face_number)->set_boundary_id(1);


Question: I like this idom of creating a const auto & ... = ... because it creates easily readable code. Shall we keep this version?

Updated to const auto center =

bangerth · 2018-06-26T06:24:14Z

examples/step-7/step-7.cc

-    static const Point<dim>   source_centers[n_source_centers];
-    static const double       width;
+    static const unsigned int                             n_source_centers = 3;
+    static const std::array<Point<dim>, n_source_centers> source_centers;


You might as well do std::array<Point<dim>,3> here, initialize it with three elements, and later on when you need to query the size do source_centers.size() (or just a range-based for loop over the elements of the array).

bangerth · 2018-06-26T06:24:59Z

examples/step-7/step-7.cc

+  const std::array<Point<1>, SolutionBase<1>::n_source_centers>
+    SolutionBase<1>::source_centers = {Point<1>(-1.0 / 3.0),
+                                       Point<1>(0.0),
+                                       Point<1>(+1.0 / 3.0)};


Could probably just brace initialize things here, unless of course you want to be explicit (which I never mind).

bangerth · 2018-06-26T06:25:20Z

examples/step-7/step-7.cc

+                                         fe_values.shape_grad(j, q_point)     //
+                                       +                                      //
+                                       fe_values.shape_value(i, q_point) *    //
+                                         fe_values.shape_value(j, q_point)) * //


same as always

bangerth · 2018-06-26T06:25:28Z

examples/step-7/step-7.cc

-              cell_rhs(i) += (fe_values.shape_value(i, q_point) *
-                              rhs_values[q_point] * fe_values.JxW(q_point));
+              cell_rhs(i) += (fe_values.shape_value(i, q_point) * //
+                              rhs_values[q_point] *               //


bangerth · 2018-06-26T06:25:34Z

examples/step-7/step-7.cc

-                       fe_face_values.JxW(q_point));
+                    cell_rhs(i) += (neumann_value *                          //
+                                    fe_face_values.shape_value(i, q_point) * //
+                                    fe_face_values.JxW(q_point));


bangerth · 2018-06-26T06:26:21Z

examples/step-7/step-7.cc

-                     1e-12))
-                  cell->face(face_number)->set_boundary_id(1);
+                {
+                  const auto &center = cell->face(face_number)->center();


no auto reference here -- center() returns an object, not a reference.

also, maybe call it face_center?

bangerth · 2018-06-26T06:26:52Z

examples/step-7/step-7.cc

@@ -1311,101 +1310,69 @@ int main()
 {
  const unsigned int dim = 2;

-  try


I think we concluded to leave these in for the moment?

bangerth · 2018-07-05T17:51:33Z

@tamiko -- ping?

tamiko · 2018-07-05T18:12:45Z

@tamiko -- ping?

*pong* Will update this PR later this week (addressing your comments) :-)

bangerth · 2018-07-22T13:06:23Z

@tamiko -- ping again?

tamiko · 2018-08-31T15:17:05Z

@bangerth pong OK - Finally enough time to work on this again.

@masterleinad, @bangerth, @drwells What is the latest consensus on the try-catch clause in main?

drwells · 2018-08-31T15:35:03Z

I think we agreed to keep the status quo to make Windows happy (i.e., keep the try-catch in main).

tamiko · 2018-08-31T15:41:24Z

😢 Noooooooooooooooooooooooooooooooooooooooooooooooo...

drwells · 2018-08-31T15:47:17Z

Its not the ugliest thing we have to do to maintain MSVC compatibility.

bangerth · 2018-08-31T17:17:34Z

I'd like to keep it too, if only so we can teach some C++.

tamiko · 2018-09-16T18:28:13Z

@masterleinad @bangerth This is now updated. Please have a look

tamiko · 2018-09-19T18:07:51Z

@dealii/dealii ping 😺

masterleinad · 2018-09-19T18:36:58Z

examples/step-7/step-7.cc

-                                                       endc =
-                                                         triangulation.end();
-            for (; cell != endc; ++cell)
+            for (const auto cell : triangulation.cell_iterators())


Would you mind using const auto& cell here?

Well "cell" is an iterator and cell_iterators and related return an iterator range. So using auto &cell, or auto cell doesn't make a difference here. I don't mind changing it to const auto &cell but I guess @bangerth has some strong opinions on that matter. (He asked me to replace a const auto &center = ... by const auto center = ... because the right hand side returned a value not a refererence.)

I see that we used const auto &cell everywhere in the examples. So let me unify it.

Yes, I really don't have much of an opinion in this context.

masterleinad · 2018-09-19T18:38:46Z

examples/step-7/step-7.cc

-    SolutionBase<2>::source_centers[SolutionBase<2>::n_source_centers] =
-      {Point<2>(-0.5, +0.5), Point<2>(-0.5, -0.5), Point<2>(+0.5, -0.5)};
+  const std::array<Point<2>, 3> SolutionBase<2>::source_centers =
+    {Point<2>(-0.5, +0.5), Point<2>(-0.5, -0.5), Point<2>(+0.5, -0.5)};


At other places, we usually omit Point<dim> here, but I don't have a strong opinion about this.

tamiko · 2018-09-19T19:06:38Z

@masterleinad I have updated this PR

masterleinad · 2018-09-19T20:24:59Z

/home/bob/source/examples/step-7/step-7.cc:126:6: error: no viable conversion from 'double' to 'dealii::Point<1, double>'
    {(-1.0 / 3.0), (0.0), (+1.0 / 3.0)};

you need braces instead of the round brackets.

tamiko · 2018-09-19T22:06:35Z

@masterleinad I reverted back to the explicit initializer list.

masterleinad · 2018-09-19T22:09:38Z

Still fine with me.

masterleinad · 2018-09-19T22:09:56Z

/run-tests

masterleinad · 2018-09-20T03:09:22Z

/Users/cib-osx/auto-dealii-tjhei-osx/dealii/examples/step-7/step-7.cc:126:6: error: suggest braces around initialization of subobject [-Werror,-Wmissing-braces]
    {Point<1>(-1.0 / 3.0), Point<1>(0.0), Point<1>(+1.0 / 3.0)};
     ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
     {                                                        }
/Users/cib-osx/auto-dealii-tjhei-osx/dealii/examples/step-7/step-7.cc:132:6: error: suggest braces around initialization of subobject [-Werror,-Wmissing-braces]
    {Point<2>(-0.5, +0.5), Point<2>(-0.5, -0.5), Point<2>(+0.5, -0.5)};
     ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
     {                                                               }

tamiko · 2018-09-20T03:48:21Z

@masterleinad sigh What exact clang version is this AppleClang again? I do not get a warning with gcc-8, or clang-6. Anyway, double braces it is.

masterleinad · 2018-09-20T07:55:05Z

What exact clang version is this AppleClang again?

Should be llvm-3.9 or llvm-4.0.

tamiko mentioned this pull request Jun 15, 2018

Update example steps #6774

Closed

61 tasks

tamiko commented Jun 15, 2018

View reviewed changes

tamiko force-pushed the update-step-7 branch from 171a560 to 81900a5 Compare June 16, 2018 00:00

tamiko added the WIP ⚠️ label Jun 18, 2018

davydden added the Tutorials label Jun 20, 2018

bangerth reviewed Jun 26, 2018

View reviewed changes

tamiko force-pushed the update-step-7 branch from 81900a5 to ea2f681 Compare September 16, 2018 18:15

tamiko removed the WIP ⚠️ label Sep 16, 2018

masterleinad reviewed Sep 19, 2018

View reviewed changes

tamiko added 8 commits September 19, 2018 14:02

step-7: Use std::array instead of C-style array

13c083f

step-7: make bilinear forms more readable

552d504

step-7: Use foreach loop and make a code snippet more readable

a8662c5

step-7: Simplify array type and use range-based for loops

9d00c82

step-7: Use AffineConstraints instead of ConstraintMatrix

ae75ee0

step-7: address comments

02dba03

step-7: update text

7ffa767

step-7: Use const auto &cell instead of const auto cell

286f4e9

tamiko force-pushed the update-step-7 branch from b823699 to ebfba63 Compare September 19, 2018 19:06

masterleinad approved these changes Sep 19, 2018

View reviewed changes

masterleinad added the Reviewed and ready to merge label Sep 19, 2018

tamiko force-pushed the update-step-7 branch from ebfba63 to 286f4e9 Compare September 19, 2018 22:05

make apple-clang happy

1f60706

masterleinad merged commit 4a2b8fd into dealii:master Sep 20, 2018

masterleinad deleted the update-step-7 branch September 20, 2018 07:55

masterleinad restored the update-step-7 branch September 20, 2018 07:56

bangerth mentioned this pull request Sep 22, 2018

Remove a reference to GCC 3.4. #7223

Merged

tamiko mentioned this pull request May 11, 2019

modernize step 6 #8102

Merged

tamiko deleted the update-step-7 branch August 7, 2019 18:08

example/step-7: Update indenting and modernize #6781

example/step-7: Update indenting and modernize #6781

Conversation

tamiko commented Jun 15, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bangerth commented Jul 5, 2018

tamiko commented Jul 5, 2018 via email

bangerth commented Jul 22, 2018

tamiko commented Aug 31, 2018

drwells commented Aug 31, 2018

tamiko commented Aug 31, 2018

drwells commented Aug 31, 2018

bangerth commented Aug 31, 2018

tamiko commented Sep 16, 2018

tamiko commented Sep 19, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tamiko commented Sep 19, 2018

masterleinad commented Sep 19, 2018 • edited

tamiko commented Sep 19, 2018

masterleinad commented Sep 19, 2018

masterleinad commented Sep 19, 2018

masterleinad commented Sep 20, 2018

tamiko commented Sep 20, 2018

masterleinad commented Sep 20, 2018

masterleinad commented Sep 19, 2018 •

edited