Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve custom quantities API #424

Open
efaulhaber opened this issue Mar 5, 2024 · 2 comments
Open

Improve custom quantities API #424

efaulhaber opened this issue Mar 5, 2024 · 2 comments

Comments

@efaulhaber
Copy link
Member

Currently, custom quantities for trixi2vtk, SolutionSavingCallback and PostprocessCallback must be functions of (v, u, t, system).
The problem with that is that one needs to use internal, undocumented functions like each_moving_particle, particle_pressure, current_coords and so on to properly work with v and u.

I therefore changed the docs in #423 to mention that functions must have the arguments (v, u, t, system), but I also mentioned that these required undocumented internal functions to work with. I also removed the examples defining such functions and instead used the pre-defined custom quantities like kinetic_energy.

I see the following options:

  1. Leave the API as is, add more pre-defined quantities, and then add a docs page explaining internals like each_moving_particle, particle_pressure, current_coords and so on. We should probably have such a page anyway, as it's very useful for users who want to develop TrixiParticles.
  2. Allow custom quantity functions to either have the arguments (v, u, t, system) (which is then non-public API) or (coordinates, velocity, density, pressure), similar to how the source terms work, but passing global arrays instead of per-particle quantities. Note, however, that we don't want to allocate these global arrays, so we'll have to pass views, which can be modified by the user, causing unpredictable behavior. This option definitely requires less knowledge about TrixiParticle internals to be able to write simple custom quantity functions.
@efaulhaber
Copy link
Member Author

To be able to use our callbacks for more complicated things that require data from multiple systems, like interpolation, we also need an option to pass a function with the arguments v, u, t, v_ode, u_ode, semi. I think there is no doubt that this should be internal and not public API, since the whole wrapping stuff is also not public API.

So,

  1. Leave the public API as is, add internal API that uses arguments v, u, t, system, v_ode, u_ode, semi (same as public API, but with v_ode, u_ode, semi), add more pre-defined quantities, and then add a docs page explaining internals like each_moving_particle, particle_pressure, current_coords and so on. We should probably have such a page anyway, as it's very useful for users who want to develop TrixiParticles.
  2. Allow custom quantity functions to either have the arguments (v, u, t, system, v_ode, u_ode, semi) (which is then non-public API) or (coordinates, velocity, density, pressure), similar to how the source terms work, but passing global arrays instead of per-particle quantities. Note, however, that we don't want to allocate these global arrays, so we'll have to pass views, which can be modified by the user, causing unpredictable behavior. This option definitely requires less knowledge about TrixiParticle internals to be able to write simple custom quantity functions.

@efaulhaber
Copy link
Member Author

I think we (@LasNikas @svchb) discussed a while ago that a good option would be something similar to point 2 above:

  1. Allow custom quantity functions to either have the arguments (v, u, t, system, v_ode, u_ode, semi) (which is then non-public API) or (data), which is a named tuple containing quantities like coordinates, velocity, density and pressure, but it depends on the system type. Users can check which quantities are available by calling a function available_quantities(system) or so. This solves the big problem of point 2 that density and pressure are not available for every system. The note about the views in point 2 still stands, though.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant