Add support for "user" output

[pgrete]

PR Summary

After some back and forth, I decided to actually open a PR for this given the small amount of change.
This adds support for completely custom user output (by downstream codes).
We've been using this in AthenaPK to write some "per block" statistics.
While I'm aware that this is not the most flexible/clean approach (given that it's hardcoded to a single UserOutput class there's only support for a single type of user output at the moment), I think it's useful to have.
A more flexible approach would certainly be a move to a package based output system, which would be a bigger overhaul.

Following @Yurlungur comments I now went with the more clean approach, i..e, this PR adds support for downstream apps to enroll an arbitrary number of custom user outputs that can do whatever they want but are triggered and controlled the same way as other outputs.

PR Checklist

• Code passes cpplint
• New features are documented.
• Adds a test for any bugs fixed. Adds tests for new features.
• Code is formatted
• Changes are summarized in CHANGELOG.md
• Change is breaking (API, behavior, ...)
  • Change is additionally added to CHANGELOG.md in the breaking section
  • PR is marked as breaking
  • Short summary API changes at the top of the PR (plus optionally with an automated update/fix script)
• CI has been triggered on Darwin for performance regression tests.
• Docs build
• (@lanl.gov employees) Update copyright on changed files

[Yurlungur]

I'm not sure I fully understand this actually. Is "user" output totally free-flowing? I.e., the user is responsible for opening/managing files, etc.?

I actually don't think it would be that big of an overhaul to do this per-package... and I don't love that this is a completely compile-time hook. i.e., it's not possible to hot-swap the output at runtime at all.

[Yurlungur]

That said, if this is something you need @pgrete I'm not going to block it.

[pgrete]

I'm not sure I fully understand this actually. Is "user" output totally free-flowing? I.e., the user is responsible for opening/managing files, etc.?

Yes, the downstream code is fully responsible, see https://github.com/parthenon-hpc-lab/athenapk/blob/pgrete/next-w-tracer-latest/src/outputs/per_block.cpp on how this has been used in AthenaPK

I actually don't think it would be that big of an overhaul to do this per-package... and I don't love that this is a completely compile-time hook. i.e., it's not possible to hot-swap the output at runtime at all.

Might not be that big of an overhaul, but upcoming leave of absence, I'm trying to tie up as many loose ends as possible.
So I'm happy to put a refactor on my todo-later list when more downstream codes need this kind of functionality as I imagine it'd it be easily backwards compatible by allowing a "user" named output type, too, in the overhaul.

[Yurlungur]

👍 I don't love this approach, but I'm happy to approve it for now if it will get refactored when other codes need it.

[pgrete]

I had a brief look at a more cleaner implementation (so gauging how realistic it is to just update this PR).

My take away is that the registration itself would be fairly straightforward and that's it mostly would be around what to store in the callback dict.
Do you imagine a function would be more suitable (i.e., the WriteOutputFile function) and we create the Output object dynamically within Parthenon, or should downstream codes enroll entire Output objects in the dictionary?
In this context I also wonder what the better approach would be when we eventually refactor the output machinery as a package.

[Yurlungur]

My take away is that the registration itself would be fairly straightforward and that's it mostly would be around what to store in the callback dict. Do you imagine a function would be more suitable (i.e., the WriteOutputFile function) and we create the Output object dynamically within Parthenon, or should downstream codes enroll entire Output objects in the dictionary?

Hmm... I dunno. I think a simple function is easier. And that doesn't actually preclude making it a class if a downstream code wants to make it a functor.

In this context I also wonder what the better approach would be when we eventually refactor the output machinery as a package.

Yeah that might be worth doing, but that probably deserves its own MR down the line.

[pgrete]

Alright, I now updated the PR to the "cleaner" approach for registering (arb number of custom outputs) and added an example and testing.

[acreyes]

Could this be a dictionary of factories rather than storing OutputTypes?

Dictionary<std::function<std::shared_ptr<OutputType>(OutputParameters)>> user_output_factories_;

This way the user outputs could be generated newly for each input block. I think of use cases like the multiple history files generated in the advection example

[pgrete]

I thought about the factory and the decided for the full derived class approach because I thought it's more flexible, e.g., when a user want to keep some state between outputs (e.g., thinking about some ADIOS2 streaming based output).

Similarly, with the current approach multiple output of the same type are still possible, aren't they?

As far as I can tell,

<parthenon/output1>
file_type = particle_user_output
dt = 2.0
variables = bla, blub

<parthenon/output2>
file_type = particle_user_output
dn = 1
variables = mycyclevar
myothervar = foobar

should work out of the box as WriteOutputFile also contains pin, additional custom parameters could be parsed and used.

[acreyes]

In your example when you call ptype->WriteOutputFile(pm, pin, tm, signal); won't it only ever write the output2 file, regardless ? Because it is registered as

pnew_type = papp_in->GetUserOutput(op.file_type);
      if (pnew_type) {
        // Update empty OutputParams op with actual ones
        pnew_type->output_params = op;
      } else {

[pgrete]

Ah, yes, good point.
Do you think it'd be an issue if I add the option for the object to clone itself (before assigning the OutputParameters (as it'd be a smaller refactor compared to writing a factory with functions)?

[acreyes]

I think the factory would be clearer downstream, but cloning should get the same resulting behavior.

[pgrete]

Just to double check: Do you imagine the factory living in Parthenon with the downstream code just providing shared ptr to the WriteOutputFile implementation or do you imagine the downstream codes write factories that are called by Parthenon to create output objects (or something completely else)?
I think having control over the object in the downstream code would be useful as it allows for more flexibility than a "simple" callback function.

[acreyes]

I was thinking it would be something like

pman.app_input->RegisterUserOutput(
      "particle_user_output", 
[](OutputParameter op){
   return std::make_shared<particles_leapfrog::ParticleUserOutput>(op);
}
);

, which I don't think would preclude allowing the downstream code to own the object, since the factory doesn't have to always make a new object. You could even throw from the factory function if the output type gets used in multiple input blocks

[Yurlungur]

I think @acreyes 's point about factories is a good one. But I'm happy with this. Much improved!