Add diagnostic vars #119
Merged
Add diagnostic vars #119
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
mirenradia
added
the
enhancement
|
This resolves #88. |
mirenradia
force-pushed
the
enhancement/diagnostic_split
branch
2 times, most recently
from
d73c765 to
f770f7a
Compare
Merged
mirenradia
added a commit
that referenced
this pull request
May 26, 2020
This resolves #118. * The enum and name definitions of the common CCZ4 variables in the provided examples have been moved into a common CCZ4UserVariables.hpp file. * The type of UserVariables::variable_names has been changed to a more modern C++ type of std::array<std::string, NUM_VARS>. Note that this is now const rather than constexpr but I don't think this is an issue. * A new variable_name_to_enum function which only work with the above type for variable_names has been defined in the UserVariables namespace in a new UserVariables.inc.hpp. Since switching to this will break users' other examples which don't include this file yet, the old variable_name_to_enum has been temporarily retained in ChomboParameters. It should be removed in #119. * A new template function to concatenate std::arrays is defined in a new ArrayTools namespace (this really ought to be in the standard library) which is used to concatenate the CCZ4 variable names array with the user one.
mirenradia
added a commit
that referenced
this pull request
May 26, 2020
This resolves #118. * The enum and name definitions of the common CCZ4 variables in the provided examples have been moved into a common CCZ4UserVariables.hpp file. * The type of UserVariables::variable_names has been changed to a more modern C++ type of std::array<std::string, NUM_VARS>. Note that this is now const rather than constexpr but I don't think this is an issue. * A new variable_name_to_enum function which only work with the above type for variable_names has been defined in the UserVariables namespace in a new UserVariables.inc.hpp. Since switching to this will break users' other examples which don't include this file yet, the old variable_name_to_enum has been temporarily retained in ChomboParameters. It should be removed in #119. * A new template function to concatenate std::arrays is defined in a new ArrayTools namespace (this really ought to be in the standard library) which is used to concatenate the CCZ4 variable names array with the user one.
This guards many calls to m_state_diagnostics in GRAMRLevel with if (NUM_DIAGNOSTIC_VARS > 0)
mirenradia
force-pushed
the
enhancement/diagnostic_split
branch
from
f770f7a to
ef4ae0a
Compare
This is done by passing the appropriate argument when calling interpolationQuery::add_comp. Some other changes: * Remove deprecated ChomboParameter::variable_name_to_enum. * Add new DiagnosticVariables::variable_name_to_enum. * Add VariableType enum class. * Remove virtual GRAMRLevel::specificWritePlotHeader since vars in plot files are controlled by a parameter. * Add selection of diagnostic variables to be in plot files (as for normal evolution variables) rather than having all of them.
|
Looks great! As discussed, we need to remove specificWritePlotHeader from GRAMRLevel (and AMRLevel?) and we can move Ham and Mom to diagnostics. Apart from this I am happy. |
KAClough
approved these changes
Jul 1, 2020
Also remove deprecated GRAMRLevel::specificWritePlotHeader
mirenradia
force-pushed
the
enhancement/diagnostic_split
branch
from
de0f7b3 to
222e051
Compare
The wiki page is a guide on how to update old examples to be compatible with the diagnostic variables changes.
GCC v5 is a little bit more fussy when it comes to implicitly constructing std::tuples. This looks related to http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2015/n4387.html which was applied in GCC v6.1.
mirenradia
force-pushed
the
enhancement/diagnostic_split
branch
from
104cee9 to
2a2ad1f
Compare
|
This all seems to work fine in the SF example and the wiki page looks good. Sorry to everyone for breaking code, but this is a major performance enhancement, so it will be worth it! |
This was referenced Jul 5, 2020
KAClough
pushed a commit
to KAClough/GRChombo_public
that referenced
this pull request
Jul 9, 2020
This resolves GRTLCollaboration#118. * The enum and name definitions of the common CCZ4 variables in the provided examples have been moved into a common CCZ4UserVariables.hpp file. * The type of UserVariables::variable_names has been changed to a more modern C++ type of std::array<std::string, NUM_VARS>. Note that this is now const rather than constexpr but I don't think this is an issue. * A new variable_name_to_enum function which only work with the above type for variable_names has been defined in the UserVariables namespace in a new UserVariables.inc.hpp. Since switching to this will break users' other examples which don't include this file yet, the old variable_name_to_enum has been temporarily retained in ChomboParameters. It should be removed in GRTLCollaboration#119. * A new template function to concatenate std::arrays is defined in a new ArrayTools namespace (this really ought to be in the standard library) which is used to concatenate the CCZ4 variable names array with the user one.
tamaraevst
pushed a commit
to tamaraevst/GRChombo
that referenced
this pull request
Dec 2, 2021
This is the same as GRTLCollaboration#124 * The enum and name definitions of the common CCZ4 variables in the provided examples have been moved into a common CCZ4UserVariables.hpp file. * The type of UserVariables::variable_names has been changed to a more modern C++ type of std::array<std::string, NUM_VARS>. Note that this is now const rather than constexpr but I don't think this is an issue. * A new variable_name_to_enum function which only work with the above type for variable_names has been defined in the UserVariables namespace in a new UserVariables.inc.hpp. Since switching to this will break users' other examples which don't include this file yet, the old variable_name_to_enum has been temporarily retained in ChomboParameters. It should be removed in GRTLCollaboration#119. * A new template function to concatenate std::arrays is defined in a new ArrayTools namespace (this really ought to be in the standard library) which is used to concatenate the CCZ4 variable names array with the user one.
mirenradia
added a commit
to mirenradia/GRChombo-public
that referenced
this pull request
Feb 3, 2023
This resolves GRTLCollaboration#118. * The enum and name definitions of the common CCZ4 variables in the provided examples have been moved into a common CCZ4UserVariables.hpp file. * The type of UserVariables::variable_names has been changed to a more modern C++ type of std::array<std::string, NUM_VARS>. Note that this is now const rather than constexpr but I don't think this is an issue. * A new variable_name_to_enum function which only work with the above type for variable_names has been defined in the UserVariables namespace in a new UserVariables.inc.hpp. Since switching to this will break users' other examples which don't include this file yet, the old variable_name_to_enum has been temporarily retained in ChomboParameters. It should be removed in GRTLCollaboration#119. * A new template function to concatenate std::arrays is defined in a new ArrayTools namespace (this really ought to be in the standard library) which is used to concatenate the CCZ4 variable names array with the user one.
mirenradia
pushed a commit
to mirenradia/GRChombo-public
that referenced
this pull request
Feb 3, 2023
…agnostic_split Add diagnostic vars
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Diagnostic Variables work in progress - has a bug at the moment where NUM_DIAGNOSTIC_VARS=0 is not a valid option, code doesn't compile. Pull request created to discuss best solution.