Picongpu: Moving FieldTmpSolvers defintion

[pordyna]

At the time FieldTmpSolvers is defined in fileOutput.param. All used solvers have to be included in this sequence to work properly. FieldTmp is already used in features not related to file output plugins (Bremsstrahlung, ionization). I'm just working on a plugin that uses tmp fields and I find it unintuitive that it depends on a nonrelated .param file.

Should we move the declaration somewhere else? One could create a sequence of needed solvers for each plugin, that uses them, in its .param file and merge them somewhere else; maybe also use mpl::unique to avoid repetition.

[sbastrakov]

This indeed looks like an inconsistency. However, I am confused by your suggested fix: we can only merge later if the type names are fixed beforehand. So then instead of having the "keyword" FieldTmpSolvers we would end up with a few new ones with similarly fixed names and all these types have to be defined in a setup. Or did I misunderstand your idea?

[pordyna]

You are right. It's just much simpler to define all at one place in an extra param file.

[ax3l]

I do not fully understand this. The list of FieldTmpSolvers in fileOutput.param is only used for field output of in situ generated fields. The defines in there should only be used for I/O.

Everywhere else where we need FieldTmp data from selected solvers, we just do a fully independent deriveField on the fly (e.g. Thomas-Fermi ionization, ISAAC). The latter do not need to be defined in fileOutput.param and are fully independent.

Can you link an example where this is not the case? Maybe I miss something.

[pordyna]

In FieldTmp.tpp FieldTmpSolvers is used to, as I understand it, determine the required exchange size for allocating TmpFields. If we calculate an independent deriveField, as @ax3l was saying on the fly, it can happen that this previously determined, on compile time, exchange size is too small if the deriveField was not included in FieldTmpSolvers. That will result in a runtime error.

I had that happen to me. Inclusion of the derived field in FieldTmpSolvers solved the problem.
So what I see is, that a user by adjusting the fileOutput.param for his needs could potentially break the program and be left with an error.

[ax3l]

determine the required exchange size for allocating TmpFields.

I do not understand. FieldTmp is with 1 to N slots statically pre-allocated, just as all other fields (E, B, J). The same happens for its exchanges
You seam to somehow mismatch this with the constexpr bool fieldTmpSupportGatherCommunication = true; option in memory.param ... Potentially there is a static assert missing if a user changes this to non-default (false)?

In order to follow you better: Can you please provide a minimal example diff that demonstrates the problem?
Just take one of the examples and diff what you changed, which make it easier for us to reproduce what you mean.

[pordyna]

So finally I have sth to show :)
The example from Saxs PR #3134 fails to run when I remove Denisty_Seq from FieldTmpSolvers in fileOutput.param ( and remove the electron density adios output ). The simulation crashes while computing the FieldTmp values for the electron density inside the SAXS plugin.

stdout.txt
stderr.txt

[sbastrakov]

Thanks @pordyna , we will take a look.

[psychocoderHPC]

@pordyna COuld you please come with this issue tomorrow to me. It sounds like a issue we have seen already in the past.
If you call a kernel with in precompiler region which is host only cuda will not generate the kernel instance.

void example_function()
{
#ifdef __CUDA_ARCH__
  // cude is not allows to cal a kernel from such a construct. 
  kernel<<<...>>>(...);
#endif
}

Sometimes code like in my example is not obvious because the kernel call is somewhere called from another function instead directly within the guards.

[sbastrakov]

@psychocoderHPC yes, I also recalled that one when looking at the attached error. Issue #3010, PR #3015 fixed it.

[pordyna]

Moving the type definitions inside the class methods as in #3015 has fixed the issue. Thank you @psychocoderHPC @sbastrakov @ax3l for your help :smile:

[sbastrakov]

Great, thanks for reporting and closing the issue @pordyna .

[psychocoderHPC]

for the docu: here is the commit with the fix: https://github.com/ComputationalRadiationPhysics/picongpu/pull/3134/commits/c3e1d2557a132af17cdf55647a23f6af95063dcf