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

Convert center to auto prob params #1222

Merged
merged 12 commits into from
Sep 24, 2020

Conversation

maxpkatz
Copy link
Member

PR summary

PR motivation

PR checklist

  • test suite needs to be run on this PR
  • this PR will change answers in the test suite to more than roundoff level
  • all newly-added functions have docstrings as per the coding conventions
  • the CHANGES file has been updated, if appropriate
  • if appropriate, this change is described in the docs

@zingale
Copy link
Member

zingale commented Sep 22, 2020

zingale pushed a commit that referenced this pull request Sep 22, 2020
Previously you could add parameters to _prob_params that were not in the Fortran namelist. No C++ version was made of these parameters. However, there is a need for even the ones not in the namelist to have a C++ version, since we may set those variables in probinit and want them later in C++. This implements that change, with the exception of parameters whose size depends on a Fortran module parameter (since we have no direct analogue of that in C++). As a demonstration, many of the wdmerger C++ problem parameters (which were manually managed as duplicates of the corresponding Fortran parameters) are converted to this new format.

This will also be helpful for finishing up #1222 since we will need center in C++.
Conflicts:
	Source/hydro/Castro_ctu_nd.F90
@maxpkatz maxpkatz marked this pull request as ready for review September 23, 2020 06:01
@zingale
Copy link
Member

zingale commented Sep 23, 2020

@maxpkatz
Copy link
Member Author

The only failure was RadSuOlson which was due to a flaw in my logic in the probdata script (for the case with a non-zero number of parameters but none of which use the fortin namelist). This has been fixed so we can retest.

@zingale
Copy link
Member

zingale commented Sep 23, 2020

@maxpkatz
Copy link
Member Author

That crashes because the problem doesn't call probdata_init(). This is a problem because we cannot assume external setups will know to call it, so fixing it in all of the problems in Exec/ is not a real solution. I'll need to remove the dependence on the problem calling it in a separate PR before we can merge this one.

zingale pushed a commit that referenced this pull request Sep 23, 2020
Rather than rely on problems to manually call probdata_init() in amrex_probinit(), this is now called by default for all setups.

While this has no specific benefit at present (other than simplifying code), it becomes functionally relevant when we start adding parameters to _default_prob_params (#1222).
@zingale
Copy link
Member

zingale commented Sep 23, 2020

@maxpkatz
Copy link
Member Author

Sedov should be fixed

@zingale zingale merged commit 9cb3701 into AMReX-Astro:development Sep 24, 2020
@maxpkatz maxpkatz deleted the prob_param_center branch September 24, 2020 04:06
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

Successfully merging this pull request may close these issues.

None yet

2 participants