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

get rid of global / managed runtime parameters #2685

Open
5 tasks
zingale opened this issue Dec 19, 2023 · 5 comments
Open
5 tasks

get rid of global / managed runtime parameters #2685

zingale opened this issue Dec 19, 2023 · 5 comments

Comments

@zingale
Copy link
Member

zingale commented Dec 19, 2023

GPU compilers seem to have trouble with managed-memory runtime parameters, so we should remove them.

The idea is that we will have the current python script define a set of structs for each namespace and then a parent struct, params that has each namespace struct. E.g.:

struct hydro_params {

    int time_integration_method{};
    int ppm_type{};

};

struct gravity_params{

    int do_grav{};
    int grav_source_type{};

};

struct params {

    hydro_params hydro;
    gravity_params gravity;

};

We will then make params run_params a static member variable of the Castro class. This would allow us to access it anywhere.

If we want to simplify things, we could do something like:

auto & hp = run_params.hydro;

to just access the hydro parameters.

Currently all of the Castro runtime parameters are parmparsed in separate headers, like castro_queries.H that are all included in intiialize_cpp_runparams().

We can implement this a bit at a time. Here's an outline:

  • modify parse_castro_params.py to create the struct's for each namespace and the parent struct. These can all be in a single header. This is done in PR start of moving runtime parameters to structs #2688.
  • modify parse_castro_params.py to also create a "sync" function that initializes the runtime parameter structs by setting the value from the global version. This is temporary. In the *_queries.H, also set the struct version of the parameter. This is done in PR start of moving runtime parameters to structs #2688.
  • Wherever we reset runtime parameters in read_params() or Castro_setup.cpp, also update the struct
  • Convert all of the Castro functions to access the parameters from the new struct instead of the global header. We can do this a bit at a time.
  • Once all of the code is updated, remove the global managed variables
@zingale
Copy link
Member Author

zingale commented Dec 19, 2023

We will need to separately do the same for Microphysics, but we can do that after this.

@zingale
Copy link
Member Author

zingale commented Dec 19, 2023

Note: for GPU kernels, we will need to pass the parameter struct as a lambda-captured copy down through any GPU functions we call in each kernel.

@zingale
Copy link
Member Author

zingale commented Dec 19, 2023

actually, it looks like in Castro_setup.cpp we do some modifications to the parameters, so we should move the sync to there

@zingale
Copy link
Member Author

zingale commented Dec 19, 2023

most of the infrastructure is now in place for this.

zingale added a commit that referenced this issue Jan 2, 2024
@zingale
Copy link
Member Author

zingale commented Aug 1, 2024

we will be able to do this on Intel / SYCL:
AMReX-Codes/amrex#4056

we will still want to do #2701

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

No branches or pull requests

1 participant