fixes issue where external variables are not properly initialized #29520

DanielYankura · 2024-12-10T23:38:19Z

-issue was caused by external variables never being initialized
-in InitProblemAction.C it now checks if there is an external variable
-if there is then it calls initializeVariable
-initializeVariable was added to BasicExternalProblem and SyncTestExternalProblem

closes #18396

Reason

If a variable is not properly initialized then it can cause a lot of issues.

Design

Adds check to InitProblemAction that looks to see if there are any external variables. If there are then it calls the initialzeVariable function which must be defined in the desired external variable source file.

Impact

Allows users to utilize external variables for a broader range of issues.

GiudGiud · 2024-12-11T00:56:03Z

test/src/problems/BasicExternalProblem.C

+  for (const auto elem_ptr : mesh.element_ptr_range())
+  {
+    auto dof_idx = elem_ptr->dof_number(sys_number, _heat_source_var, 0);
+    solution.set(dof_idx, 12345);


I would use the ExternalProblem directly rather than those test objects to test the fix

I'm not sure this is the fix I had in mind -- we don't necessarily have an initial value to put into this external variable until we run our OpenMC simulation. The problem is that even after we run OpenMC, the user objects are lagged by one time step (with the current MOOSE, not including any changes in this PR). This fix seems to be just manually adding an insertion point to initialize the variable (which for us would mean an extra OpenMC run -- as opposed to MOOSE just realizing we already ran OpenMC and properly keeping things up to date).

Is my understanding correct?

I believe you're correct. When I looked at the two input files you included it looked like for the external case heat_source was never initialized and so it defaulted to 0. Then in the first iteration layered_integral was calculated before heat_source was ever assigned a value which is why it was off for that first timestep. That's why I assumed it was an issue with initialization.

So I did some more digging and I think I may have changed the execute_on parameter during some of my debugging but forgot to change it back. So I went back and ran the two original input files you included and when both the aux kernel and user object are set to execute on timestep_end I'm getting the same results for both. The only difference being that for the external user object heat_source is 0 for t = 0 unless I initialize heat_source. I'm wondering if this issue may have been resolved in a different PR and adding an option to initialize and external variable was the only thing left to fix? I was going to try and do some testing with a time dependent problem and see if it still works.

@aprilnovak So I ran similar problems that were time-dependent and both the external and non-external user object versions gave the same results. The only difference was that for the external user object, heat_source is initialized to 0. I think this issue may have been resolved earlier. Unfortunately I'm not sure which PR may have fixed it. For reference here are the two examples I ran. For the external one I modified the SyncSolutions function in BasicExternalProblem.C so that it set heat_source to 12345*time(). If you run into any similar issues let me know and I'll take another look!
time-depenent example.zip

GiudGiud · 2024-12-11T00:56:31Z

framework/src/actions/InitProblemAction.C

    _problem->init();
  else
    mooseError("Problem doesn't exist in InitProblemAction!");
+
+  // check for external variable to initialize
+  auto external_problem_ptr = std::dynamic_pointer_cast<ExternalProblem>(_problem);
+  if (external_problem_ptr)
+    external_problem_ptr->initializeVariable();


any chance you can move that call to initializeVariable inside the ExternalProblem init() ?

then you wont need to define these new functions to the two test objects

So the issue I would run into there is that the FEProblemBase object _problem has not been initialized yet. Do you know if there is another way I can get a pointer to the external problem object at that point?

I think it's inited you can see it right above

_problem->init();

Yes, but you wanted it moved into ExternalProblem.C right?

I guess I should clarify, it can be overridden in a derived class but the parent constructor is not going to call the overridden version.

the _problem->init call is not the constructor
you just need to redefine ExternalProblem::init to include the logic you want to add here

You can call down to the base FEProblemBase::init from within your derived ExternalProblem::init override

but sounds like we're not going to end up going down this path anyways

That's what I ended up doing. I was going to push it this morning but as you said I'm not sure that this is the right solution.

GiudGiud · 2024-12-11T00:57:18Z

framework/include/problems/ExternalProblem.h

+  /**
+   * Method to initialize variable before starting the simulation
+   */
+  virtual void initializeVariable() {}


Suggested change

virtual void initializeVariable() {}

virtual void initializeVariables() {}

GiudGiud · 2024-12-11T00:57:25Z

framework/include/problems/ExternalProblem.h

@@ -46,4 +46,9 @@ class ExternalProblem : public FEProblemBase
   * transfer to variables in Multiapp simulations.
   */
  virtual void addExternalVariables() {}
+
+  /**
+   * Method to initialize variable before starting the simulation


Suggested change

* Method to initialize variable before starting the simulation

* Method to initialize variables before starting the simulation

moosebuild · 2024-12-11T02:08:06Z

Job Documentation, step Docs: sync website on 339c477 wanted to post the following:

View the site here

This comment will be updated on new commits.

moosebuild · 2024-12-11T02:54:08Z

Job Coverage, step Generate coverage on 339c477 wanted to post the following:

Framework coverage

	38207e	#29520 339c47
	Total	Total	+/-	New
Rate	85.25%	85.25%	+0.00%	100.00%
Hits	107993	107998	+5	5
Misses	18682	18682	-	0

Diff coverage report

Full coverage report

Modules coverage

Coverage did not change

Full coverage reports

Reports

This comment will be updated on new commits.

-issue was caused by external variables never being initialized -in InitProblemAction.C it now checks if there is an external variable -if there is then it calls initializeVariable -initializeVariable was added to BasicExternalProblem and SyncTestExternalProblem closes idaholab#18396

-Reverted BasicExternalProblem and SynceTestExternalProblem -Check to external variable initialization moved to FEProblemBase.C -function name changed to initializeVariables -initializeVariables can be modified in ExternalProblem.C but can also be overriden if needed

moosebuild · 2024-12-17T20:54:34Z

Job Precheck, step Clang format on 4ce5b0a wanted to post the following:

Your code requires style changes.

A patch was auto generated and copied here
You can directly apply the patch by running, in the top level of your repository:

curl -s https://mooseframework.inl.gov/docs/PRs/29520/clang_format/style.patch | git apply -v

Alternatively, with your repository up to date and in the top level of your repository:

git clang-format 38207e829fa6e8719263c8ab300526f41a2bc77e

…revious test

DanielYankura requested a review from lindsayad as a code owner December 10, 2024 23:38

GiudGiud reviewed Dec 11, 2024

View reviewed changes

DanielYankura added 3 commits December 17, 2024 13:49

added back in newline what was deleted previously

5d775e6

DanielYankura force-pushed the external_user_object branch from b05630b to 4ce5b0a Compare December 17, 2024 20:49

DanielYankura added 2 commits December 17, 2024 13:55

style changes

cd3161a

Removed time dependence from BasicExternalProblem that I used for a p…

339c477

…revious test

GiudGiud self-assigned this Dec 20, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fixes issue where external variables are not properly initialized #29520

fixes issue where external variables are not properly initialized #29520

DanielYankura commented Dec 10, 2024

GiudGiud Dec 11, 2024

GiudGiud Dec 11, 2024

aprilnovak Dec 11, 2024

DanielYankura Dec 11, 2024

DanielYankura Dec 12, 2024

DanielYankura Dec 17, 2024

GiudGiud Dec 11, 2024

GiudGiud Dec 11, 2024

DanielYankura Dec 11, 2024 •

edited

Loading

GiudGiud Dec 11, 2024

DanielYankura Dec 11, 2024

DanielYankura Dec 11, 2024

GiudGiud Dec 11, 2024

lindsayad Dec 12, 2024

lindsayad Dec 12, 2024

DanielYankura Dec 12, 2024

GiudGiud Dec 11, 2024

GiudGiud Dec 11, 2024

moosebuild commented Dec 11, 2024 •

edited

Loading

moosebuild commented Dec 11, 2024 •

edited

Loading

moosebuild commented Dec 17, 2024

	virtual void initializeVariable() {}
	virtual void initializeVariables() {}

	* Method to initialize variable before starting the simulation
	* Method to initialize variables before starting the simulation

fixes issue where external variables are not properly initialized #29520

Are you sure you want to change the base?

fixes issue where external variables are not properly initialized #29520

Conversation

DanielYankura commented Dec 10, 2024

Reason

Design

Impact

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

DanielYankura Dec 11, 2024 • edited Loading

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

moosebuild commented Dec 11, 2024 • edited Loading

moosebuild commented Dec 11, 2024 • edited Loading

Framework coverage

Modules coverage

Full coverage reports

moosebuild commented Dec 17, 2024

DanielYankura Dec 11, 2024 •

edited

Loading

moosebuild commented Dec 11, 2024 •

edited

Loading

moosebuild commented Dec 11, 2024 •

edited

Loading