Clean up TransientMultiApp

[YaqiWang]

Rationale

Implementation of TransientMultiApp needs to be carefully examined again to make sure its robustness. Particularly its implementation should meet one single and important requirement:

An transient input driven by a dummy master should reproduce the results by running the input as a master in all circumstances.

Description

We need this to make our multiphysics coupling calculations with multiapps really plug and play. TransientMultiApp should contain minimum coding related with time stepping, integration and solving. Code duplication with Transient executioner needs to be removed completely. I did not check the code carefully but have a feeling it needs to be cleaned up throughly.

Impact

Better coding and user experience.

[YaqiWang]

Few items I can think of are listed.

1. Interpolation with sub-cycling: a) Transfer should be transparent to this. b) TransientMultiApp create one vector to store the transferred solution at target time. c) Transient should be able to detect this when running as a multiapp and do the interpolation as what we did with Picard relaxation. Thus no special treatment is going to needed in TransientMultiApp.
2. Create a function in Transient to step to a target time. In stand-alone mode, it will just step to the end, in MultiApp sub-cycling, it can step to any target time. This function will remove all duplications in the sub-cycling branch within takeStep(). We should let Transient to cut time step and re-try and detect steady-state.
3. Catch-up: If I understand correctly, Transient already has catch-up capability, so we can just remove it from TransientMultiApp.
4. Timestep selection: Anywhere the design is documented?
5. NeedsRestoration: Is the function documented anywhere? The logic is fairly complicated and seems fragile.

[YaqiWang]

TransientMultiApp::appTransferVector is doing two things, one is to return the solution vector where transferred variables are stored, another is to tell multiapp what variables are to be transferred. The second functionality is currently necessary for doing the interpolation only for transferred variables. If we can find a way to let MOOSE automatically register the transferred variables for all multiapps instead of let transfers to call the function currently (derived class could be missing the call thus introducing potential bugs), it will be great. We can then remove TransientMultiApp::appTransferVector completely and make the interpolation transparent to the transfers. @friedmud and @permcody do you have any thoughts on this?

[permcody]

Without looking at it at all, I would suggest that we do what we do for the MooseVariable interface classes. These classes are derived from in a number of objects and perform variable registration so that we know which variables are actually used in a particular computation so we don't waste time "preparing" variables that don't need to be prepared. Since the Transfer objects already take in Variables through their respective parameter objects, this should be doable.

On Mon, Feb 18, 2019 at 9:29 AM Yaqi notifications@github.com wrote:

TransientMultiApp::appTransferVector is doing two things, one is to return the solution vector where transferred variables are stored, another is to tell multiapp what variables are to be transferred. The second functionality is currently necessary for doing the interpolation only for transferred variables. If we can find a way to let MOOSE automatically register the transferred variables for all multiapps instead of let transfers to call the function currently (derived class could be missing the call thus introducing potential bugs), it will be great. We can then remove TransientMultiApp::appTransferVector completely and make the interpolation transparent to the transfers. @friedmud https://github.com/friedmud and @permcody https://github.com/permcody do you have any thoughts on this?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/idaholab/moose/issues/12742#issuecomment-464799886, or mute the thread https://github.com/notifications/unsubscribe-auth/AC5XIOCDenLR-hojS6hG8D5Tub0KlxI3ks5vOtTxgaJpZM4aLpaS .

[YaqiWang]

Unfortunately, this is different in transfers if I understand correctly. For example of MultiAppNearestNodeTransfer, there are two input parameters, variable and source_variable. They are all in type of VariableName with addRequiredParam and where they are residing depends on the transfer direction. This looks to me that using MooseVariable interface classes may not be a solution.