list registered variables when exceeding MoL accumulator limit

Comments (10)

1. 

   Roland Haas reporter

   • changed status to open
   • removed comment

   • 2013-02-14T19:18:20+00:00
2. 

   Erik Schnetter

   • removed comment

   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.

   • 2013-02-14T21:38:13+00:00
3. 

   Roland Haas reporter

   • removed comment

   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.

   • 2013-02-14T23:08:11+00:00
4. 

   Erik Schnetter

   • changed status to open
   • removed 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.

   • 2013-02-15T07:30:57+00:00
5. 

   Roland Haas reporter

   • changed status to resolved
   • removed 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.

   • 2013-02-15T13:01:06+00:00
6. 

   Frank Löffler

   • removed comment

   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, ...)
   

   • 2013-02-15T15:20:35+00:00
7. 

   Roland Haas reporter

   • removed comment

   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 strdup(s.str().c_str());
   

   • 2013-02-15T15:27:35+00:00
8. 

   Frank Löffler

   • removed comment

   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.

   • 2013-02-15T15:51:45+00:00
9. 

   Roland Haas reporter

   • removed comment

   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 eager about rewriting this several times unless it is buggy. Whatever works is fine with me :-).

   I'd much rather invest time and rewrite the whole registration file in C++ to reduce the almost duplicated code (or to get rid of the need to count the number of evolved variables altogether). For this I would be waiting until after the IMEX and other MoL changes that we have outstanding are incorporated I think.

   • 2013-02-15T17:05:30+00:00
10. 

    Roland Haas reporter

    • edited description
    • changed status to closed

    • 2019-02-21T20:07:40+00:00
11. Log in to comment