Fixed "fall through" for PR "New PR: Eigen system supports shell matrices" #15324

fdkong · 2020-05-20T17:44:48Z

/opt/civet/build_0/moose/framework/src/utils/SlepcSupport.C:385:40: error: this statement may fall through [-Werror=implicit-fallthrough=]
  385 |       solver_params._eigen_matrix_free = true;
      |       ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~
/opt/civet/build_0/moose/framework/src/utils/SlepcSupport.C:387:5: note: here
  387 |     case Moose::EST_NEWTON:
      |     ^~~~
cc1plus: all warnings being treated as errors

We changed APIs in #15315, and we need to reserve the old APIs for Griffin for a second.

I will go to update Griffin to use new APIs. Once that is done, we could remove the old APIs.

Refs #15315

/opt/civet/build_0/moose/framework/src/utils/SlepcSupport.C:385:40: error: this statement may fall through [-Werror=implicit-fallthrough=] 385 | solver_params._eigen_matrix_free = true; | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~ /opt/civet/build_0/moose/framework/src/utils/SlepcSupport.C:387:5: note: here 387 | case Moose::EST_NEWTON: | ^~~~ cc1plus: all warnings being treated as errors We changed APIs in idaholab#15315, and we need to reserve the old APIs for Griffin for a second. I will go to update Griffin to use new APIs. Once that is done, we could remove the old APIs. Refs idaholab#15315

lindsayad

What target did this fail on? Make sure that it's running on this PR

lindsayad · 2020-05-20T18:07:48Z

framework/src/utils/SlepcSupport.C

-    case Moose::EST_MF_MONOLITH_NEWTON: // This should be removed once RattleSnake is updated
-      /* Falls through. */
+    // This should be removed once Griffin is updated
+    case Moose::EST_MF_MONOLITH_NEWTON:
+      Moose::PetscSupport::setSinglePetscOption("-init_eps_monitor_conv");
+      Moose::PetscSupport::setSinglePetscOption("-init_eps_monitor");
+      Moose::PetscSupport::setSinglePetscOption("-eps_power_snes_monitor");
+      Moose::PetscSupport::setSinglePetscOption("-eps_power_ksp_monitor");
+      Moose::PetscSupport::setSinglePetscOption("-init_eps_power_snes_monitor");
+      Moose::PetscSupport::setSinglePetscOption("-init_eps_power_ksp_monitor");
+      break;
    case Moose::EST_NEWTON:


This is an empty case statement; this is fine to "fallthrough" on. The compiler is not warning/erroring about this, is it?

Yes, case Moose::EST_MF_MONOLITH_NEWTON should be empty in theory since it will take the same thing as case Moose::EST_NEWTON:

But complier is angry at me if I leave case Moose::EST_MF_MONOLITH_NEWTON empty

https://civet.inl.gov/job/530953/

We do not need to worry about this code. It will be removed once I get Griffin updated. But we need to get the moose changes in first

the compiler is only warning about the below case; not this one

But since it's temporary, I won't make you update the PR

Yes, we do not need to worry about this temporary code. I will remove it soon

lindsayad · 2020-05-20T18:08:52Z

framework/src/utils/SlepcSupport.C

-    case Moose::EST_MF_MONOLITH_NEWTON: // This block should be removed once RattleSnake is updated
+    // This block (EST_MF_MONOLITH_NEWTON) should be removed once Griffin is updated
+    // This is used to for API update
+    case Moose::EST_MF_MONOLITH_NEWTON:
      solver_params._eigen_matrix_free = true;
-      /* Falls through. */
+#if !SLEPC_VERSION_LESS_THAN(3, 8, 0) || !PETSC_VERSION_RELEASE
+      Moose::PetscSupport::setSinglePetscOption("-eps_type", "power");
+      Moose::PetscSupport::setSinglePetscOption("-eps_power_nonlinear", "1");
+      Moose::PetscSupport::setSinglePetscOption("-eps_power_update", "1");
+      Moose::PetscSupport::setSinglePetscOption("-init_eps_power_snes_max_it", "1");
+      Moose::PetscSupport::setSinglePetscOption("-init_eps_power_ksp_rtol", "1e-2");
+      Moose::PetscSupport::setSinglePetscOption(
+          "-init_eps_max_it", stringify(params.get<unsigned int>("free_power_iterations")));
+      Moose::PetscSupport::setSinglePetscOption("-eps_target_magnitude", "");
+      if (solver_params._eigen_matrix_free)
+      {
+        Moose::PetscSupport::setSinglePetscOption("-eps_power_snes_mf_operator", "1");
+        Moose::PetscSupport::setSinglePetscOption("-init_eps_power_snes_mf_operator", "1");
+      }
+
+      if (solver_params._customized_pc_for_eigen)
+      {
+        Moose::PetscSupport::setSinglePetscOption("-eps_power_pc_type", "moosepc");
+        Moose::PetscSupport::setSinglePetscOption("-init_eps_power_pc_type", "moosepc");
+      }
+#else
+      mooseError("Newton-based eigenvalue solver requires SLEPc 3.7.3 or higher");
+#endif
+      break;
    case Moose::EST_NEWTON:


Yea, this guy is not empty, so the fallthrough warning/error is valid.

fdkong · 2020-05-20T18:32:48Z

I need to fix this as well. Why PETSc-3.11.4 hates me so much?!

https://civet.inl.gov/job/530947/

moosebuild · 2020-05-20T19:29:20Z

Job Documentation on 372faa6 wanted to post the following:

View the site here

This comment will be updated on new commits.

fdkong · 2020-05-20T20:16:37Z

I can not reproduce the issue.

I keep working on

… there is no off diag coupling.

andrsd · 2020-05-21T01:08:57Z

Just FYI, the fix for the doco target is already in next, so it is safe to ignore the failure. We need a clean next so that all the fixes get into master and we can continue rolling...

fdkong · 2020-05-21T01:10:49Z

Just FYI, the fix for the doco target is already in next, so it is safe to ignore the failure. We need a clean next so that all the fixes get into master and we can continue rolling...

Good to know! Otherwise I can not believe I just broke docs :-)

moosebuild · 2020-05-21T02:26:35Z

Job Modules debug PETSc alt on 372faa6 : invalidated by @fdkong

Why it takes forever?

moosebuild · 2020-05-21T02:27:06Z

Job Modules debug PETSc submodule on 372faa6 : invalidated by @fdkong

Why it takes forever?

moosebuild · 2020-05-21T02:27:28Z

Job Pthreads+OpenMP Test on 372faa6 : invalidated by @fdkong

Why it takes forever?

fdkong · 2020-05-21T04:22:57Z

@lindsayad It is ready for review or merge

fdkong changed the title ~~Fixed "fall through" for PR #15315~~ Fixed "fall through" for PR "New PR: Eigen system supports shell matrices" May 20, 2020

fdkong requested a review from lindsayad May 20, 2020 17:46

lindsayad reviewed May 20, 2020

View reviewed changes

fdkong force-pushed the matrix_free_eigenvalue branch 2 times, most recently from 951e97a to d4e343e Compare May 20, 2020 22:44

PhysicsBasedPreconditioner: We need to close the vector properly when…

372faa6

… there is no off diag coupling.

fdkong force-pushed the matrix_free_eigenvalue branch from d4e343e to 372faa6 Compare May 20, 2020 23:17

fdkong added the PR: Ready for review/merge label May 21, 2020

lindsayad approved these changes May 21, 2020

View reviewed changes

lindsayad merged commit 1581046 into idaholab:next May 21, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fixed "fall through" for PR "New PR: Eigen system supports shell matrices" #15324

Fixed "fall through" for PR "New PR: Eigen system supports shell matrices" #15324

fdkong commented May 20, 2020

lindsayad left a comment

lindsayad May 20, 2020

fdkong May 20, 2020

fdkong May 20, 2020

lindsayad May 20, 2020

lindsayad May 20, 2020

fdkong May 20, 2020

lindsayad May 20, 2020

fdkong May 20, 2020

fdkong commented May 20, 2020

moosebuild commented May 20, 2020 •

edited

Loading

fdkong commented May 20, 2020 •

edited

Loading

andrsd commented May 21, 2020

fdkong commented May 21, 2020

moosebuild commented May 21, 2020

moosebuild commented May 21, 2020

moosebuild commented May 21, 2020

fdkong commented May 21, 2020

Fixed "fall through" for PR "New PR: Eigen system supports shell matrices" #15324

Fixed "fall through" for PR "New PR: Eigen system supports shell matrices" #15324

Conversation

fdkong commented May 20, 2020

lindsayad left a 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

fdkong commented May 20, 2020

moosebuild commented May 20, 2020 • edited Loading

fdkong commented May 20, 2020 • edited Loading

andrsd commented May 21, 2020

fdkong commented May 21, 2020

moosebuild commented May 21, 2020

moosebuild commented May 21, 2020

moosebuild commented May 21, 2020

fdkong commented May 21, 2020

moosebuild commented May 20, 2020 •

edited

Loading

fdkong commented May 20, 2020 •

edited

Loading