[Einstein Toolkit] #1260: list registered variables when exceeding MoL accumulator limit
#1260: list registered variables when exceeding MoL accumulator limit -----------------------------------+---------------------------------------- Reporter: rhaas | Owner: Type: enhancement | Status: new Priority: minor | Milestone: Component: EinsteinToolkit thorn | Version: Keywords: MoL | -----------------------------------+---------------------------------------- Currently when a thorn tries to register more evolved/constrained/sandr variables than the respective parameters allow, MoL aborts with an error message "You have tried to register more evolved variables than the accumulator parameter MoL_Num_Evolved_Variables allows. Check that you are accumulating onto this parameter correctly." which is not terribly helpful in finding out why this is happening.
The attached patch increases the verbosity by outputting the name of the variable that the thorn is trying to register as well as the list of variables already registered.
The patch itself is simple (if you ignore realloc() and the fact that MoL/Register.c contains the same code 12 times for all combinations of evolved/constrained/sandr gridfunction/gridarray real/complex). However this touches MoL which is rather central.
#1260: list registered variables when exceeding MoL accumulator limit ------------------------------------+--------------------------------------- Reporter: rhaas | Owner: Type: enhancement | Status: review Priority: minor | Milestone: Component: EinsteinToolkit thorn | Version: Resolution: | Keywords: MoL ------------------------------------+--------------------------------------- Changes (by rhaas):
* status: new => review
#1260: list registered variables when exceeding MoL accumulator limit ------------------------------------+--------------------------------------- Reporter: rhaas | Owner: Type: enhancement | Status: review Priority: minor | Milestone: Component: EinsteinToolkit thorn | Version: Resolution: | Keywords: MoL ------------------------------------+---------------------------------------
Comment (by eschnett):
I recommend either using Util_asnprintf instead of snprintf/realloc, or having the routine print the variable names directly to stdout before calling CCTK_WARN instead of generating a string.
#1260: list registered variables when exceeding MoL accumulator limit ------------------------------------+--------------------------------------- Reporter: rhaas | Owner: Type: enhancement | Status: review Priority: minor | Milestone: Component: EinsteinToolkit thorn | Version: Resolution: | Keywords: MoL ------------------------------------+---------------------------------------
Comment (by rhaas):
Hmm, I believe I cannot use Util_asnprintf directly since I don't know at compile time how many variables there might be so I cannot build its format string. I would either have to build the format string dynamically in memory which I could do without a realloc, or I run the list of variables twice first counting how much memory I need, then filling the string. I dislike printing to stdout directly since Cactus supports listeners for warnings (via CCTK_WarnCallbackRegister), it seems, which would be circumvented by printing to stdout directly.
Attached is an updated patch with simplified logic and no more realloc. It counts how much space will be needed, then gets memory, then uses snprintf to build the string as before.
#1260: list registered variables when exceeding MoL accumulator limit ------------------------------------+--------------------------------------- Reporter: rhaas | Owner: Type: enhancement | Status: reviewed_ok Priority: minor | Milestone: Component: EinsteinToolkit thorn | Version: Resolution: | Keywords: MoL ------------------------------------+--------------------------------------- Changes (by eschnett):
* status: review => reviewed_ok
Comment:
I don't understand your argument agains Util_asnprintf. You would call it multiple times, instead of snprintf, the only difference would be that you don't have to call malloc/realloc yourself.
You could also add a C++ file that uses a string stream.
However, the routine is small, and the complex logic is contained in it. Either way to generate a string is fine.
CCTK_FullName returns a char*, not a const char*. You should change the variable declaration. This will also remove the need for the void* cast in the call to free.
I would move the declaration of off to just before the second loop.
#1260: list registered variables when exceeding MoL accumulator limit ------------------------------------+--------------------------------------- Reporter: rhaas | Owner: Type: enhancement | Status: closed Priority: minor | Milestone: Component: EinsteinToolkit thorn | Version: Resolution: fixed | Keywords: MoL ------------------------------------+--------------------------------------- Changes (by rhaas):
* status: reviewed_ok => closed * resolution: => fixed
Comment:
I am not sure how to use Util_asnprintf when the number of conversions required is not known at compile time. I only (ab)use sprintf to produce either "thorn::varname" or " thorn::varname" and append it to the existing string. With asnprintf I would write something like asnprintf("%s %s %s", varname1, varname2, varname3) if I had three variables. But since I don't know the number I don't know how to build the argument list.
applied the cleaned up patch ".2" as rev 191 of MoL.
#1260: list registered variables when exceeding MoL accumulator limit ------------------------------------+--------------------------------------- Reporter: rhaas | Owner: Type: enhancement | Status: closed Priority: minor | Milestone: Component: EinsteinToolkit thorn | Version: Resolution: fixed | Keywords: MoL ------------------------------------+---------------------------------------
Comment (by knarf):
Wouldn't you use Util_asnprintf the same way as snprintf? The number of arguments is not required at compiler time either for that: {{{ int Util_asnprintf(char **buffer, size_t size, const char *fmt, ...) }}} which is so similar to {{{ int snprintf(char *str, size_t size, const char *format, ...) }}}
#1260: list registered variables when exceeding MoL accumulator limit ------------------------------------+--------------------------------------- Reporter: rhaas | Owner: Type: enhancement | Status: closed Priority: minor | Milestone: Component: EinsteinToolkit thorn | Version: Resolution: fixed | Keywords: MoL ------------------------------------+---------------------------------------
Comment (by rhaas):
That would not help me. I want a single string that contains all the variable names. I get (from MoL) an array of variable indices. I want to turn it into a single string listing all the variable names, separated by spaces, terminated but a '\0'.
I want what you would get form this C++ code: {{{ ostringstream s; for(int i = 0 ; i < numvars ; i++) { char* varname = CCTK_FullName(VarIndex[i]); if(i > 0) s << " "; s << varname; } return s.str().c_str(); }}}
#1260: list registered variables when exceeding MoL accumulator limit ------------------------------------+--------------------------------------- Reporter: rhaas | Owner: Type: enhancement | Status: closed Priority: minor | Milestone: Component: EinsteinToolkit thorn | Version: Resolution: fixed | Keywords: MoL ------------------------------------+---------------------------------------
Comment (by knarf):
Erik already mentioned it: you would call it multiple times, once for each variable - as you do with the "s< varname in the C++ code.
#1260: list registered variables when exceeding MoL accumulator limit ------------------------------------+--------------------------------------- Reporter: rhaas | Owner: Type: enhancement | Status: closed Priority: minor | Milestone: Component: EinsteinToolkit thorn | Version: Resolution: fixed | Keywords: MoL ------------------------------------+---------------------------------------
Comment (by rhaas):
As in {{{ char* retval = CCTK_FullName(VarIndex[0]); for(int i = 1 ; i < numvars ; i++) { char* newstring, *varname; varname = CCTK_FullName(VarIndex[i]); Util_asprintf(&newstring, "%s %s", retval, varname); free(varname); free(retval); retval = newstring; } return retval; }}}
? That would be closest to what the C++ code does (still needs some special logic for the numvars == 0 case). I am not sure if I find this simpler. All of this is lots of discussion about some rather simple code that is not crucial for performance and located in a single self contained function, so I am not terribly excited about rewriting this several times. Whatever works is fine with me :-).
trac@lists.einsteintoolkit.org
-
Einstein Toolkit