- removed comment
parameter file parser fails to parse parameter name
Issue #1290
closed
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)
-
reporter
-
- 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).
-
- 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.
-
- removed comment
- changed watchers to stevenrbrandt
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.
-
- 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"
-
- changed status to open
- removed comment
-
- changed milestone to ET_2013_05
- removed comment
-
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 ;-) ).
-
reporter
- changed status to open
- assigned issue to
- removed comment
-
- 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.
-
reporter
- changed status to open
- removed comment
The attached patch fixes the reported issue, the patch in
#1328would fix the error that now shows up (unless one changes the test parfile). Please apply. -
- changed status to resolved
- removed comment
-
reporter
- edited description
- changed status to closed
- Log in to comment
- Assignee
- Type
- Priority
- Status
- closed
- Component
- Cactus
- Milestone
- ET_2013_05
- Version
- –
- Votes
- 0
- Watchers
- 0
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
fails, but
works.