parameter file parser fails to parse parameter name

Create issue
Issue #1290 closed
Roland Haas created an issue

the attached parameter file (the rotor test from the ET MHD paper) fails with the current parameter parser:

Activating thorn Cactus...Success -> active implementation Cactus
ERROR IN PARAMETER FILE:
In rule 'file::set::skipeol' Line=35

SpaceMask::use_mask="yes"
EOS_Omni::gl_gamma=5./3.
# try to mimic the core fluid
EOS_Omni::poly_k = grhydro_initdata::rotor_pressin/grhydro_initdata::rotor_rhoin**EOS_Omni::gl_gamma
                                                                   ^
Expected one of the following characters: @a-zA-Z0-9_[*/%+\-<>!=&| \t\r#\n
WARNING level 0 in thorn Cactus processor 0 host horizon.tapir.caltech.edu
  (line 167 of /mnt/data/rhaas/postdoc/gr/Zelmani/src/main/ProcessParameterDatabase.c): 
  -> CCTKi_SetParameterSetMask: 1 parsing errors in parameter file
WARNING level 0 in thorn Cactus processor 0 host horizon.tapir.caltech.edu
  (line 167 of /mnt/data/rhaas/postdoc/gr/Zelmani/src/main/ProcessParameterDatabase.c): 
  -> CCTKi_SetParameterSetMask: 1 parsing errors in parameter file
--------------------------------------------------------------------------
MPI_ABORT was invoked on rank 0 in communicator MPI_COMM_WORLD 
with errorcode 0.

Keyword:

Comments (13)

  1. Roland Haas reporter
    • removed comment

    apparently the parser requires whitespace between operators and parameter name. From the error text it even seems as if it tries to accept "/" as part of the parameter name. Cactus already limits that allowed letters in parameter names to [A-Za-z]{A-Za-z0-9_] [plus the :: to separate thorn from parameter part] so that (possibly with more stuff for [] of parameter vectors) would seem like a sufficiently large class to include. having to have to separate operators from variable names by white space is unheard of in any programming language that I have ever worked with (even Tcl which allows anything* incl "/" and the empty string for variable names will require that it is referred to as ${/} when its value is needed).

    So

    EOS_Omni::poly_k = grhydro_initdata::rotor_pressin/grhydro_initdata::rotor_rhoin**EOS_Omni::gl_gamma
    

    fails, but

    EOS_Omni::poly_k = grhydro_initdata::rotor_pressin / grhydro_initdata::rotor_rhoin ** EOS_Omni::gl_gamma
    

    works.

  2. Erik Schnetter
    • removed comment

    Lisp. One of the oldest languages there is. Almost any character can be part of an identifier.

    IIRC, Steve added "/" as character that can be used in identifiers to work around a backward compatibility issue (probably a string containing a slash was used without quotes).

  3. Ian Hinder
    • removed comment

    In Lisp, +, -, *, / etc are all functions, so function names have to be able to contain such characters. In Cactus though, we tend to use language constructs which resemble C and Perl (e.g. CCL files), so we should echo what is done in those languages, which is to have a more restrictive set of allowed symbols.

  4. Erik Schnetter

    I was never arguing for making the syntax more Lisp-like.

    In fact, I am arguing for following (for example) Python's more closely, with a few obvious extensions for Cactus (e.g. handle :: to separate thorn/parameter names etc.). Defining our own syntax will only confuse people and make things more complex.

  5. Steven R. Brandt
    • removed comment

    Sorry I didn't notice this sooner. The problem was a faulty definition of the name rule. It included the / character. After removing that character, the ET test suite still runs fine.

    Index: Call.cc

    --- Call.cc (revision 4991) +++ Call.cc (working copy) @@ -722,7 +722,7 @@ "# Note that / occurs in some par files. It is my\n" "# feeling that this should require quote marks.\n"

    • "name = [@a-zA-Z_][@/a-zA-Z0-9_-]*\n"
    • "name = [@a-zA-Z_][@a-zA-Z0-9_-]\n" "dname = [0-9][a-zA-Z_]{2,}\n" "inquot = ({var}|\\.|[^\\\"])\n" "fname = \.?/[-\./0-9a-zA-Z_]+\n"
  6. Roland Haas reporter
    • changed status to open
    • removed comment

    Has this been applied? The parfile still fails for me and looking at my Cactus checkout (rev 4997) Call.cc still contains the incorrect regex. Note that my personal choice for name would be:

    name = [a-zA-Z_][a-zA-Z0-9_]*
    

    in any case, ie. a valid C identifier (allowing "" as the first character is optional I guess since initial "" is reserved for the standard library). Is there a reason why we would need to allow the "@" in variable names? Or the minus sign "-" (Erik likes "-" better than "_" I understand but that should likely not be the deciding reason if it makes parsing harder otherwise and/or generates identifiers that cannot be mapped to C/Fortran/Shell variable names ;-) ).

  7. Steven R. Brandt
    • changed status to open
    • removed comment

    So this version uses Roland's personal choice above.

    For the sake of backwards compatibility with par files that use /'s and -'s, I do the following: if we have string1 / string2, I emit a warning and produce a literal "string1/string2". I do the same thing for the "-" operator.

  8. Roland Haas reporter
    • changed status to open
    • removed comment

    The attached patch fixes the reported issue, the patch in #1328 would fix the error that now shows up (unless one changes the test parfile). Please apply.

  9. Log in to comment