#1881: Unclear error message for parameter file error ----------------------+----------------------------------------------------- Reporter: eschnett | Owner: Type: defect | Status: new Priority: minor | Milestone: Component: Cactus | Version: development version Keywords: | ----------------------+----------------------------------------------------- I had this line in a parameter file {{{ ML_BSSN_FH_Helper::ML_BSSN_FH_CalculateRHSInMoLPostStep }}} which is missing the {{{= yes}}} at the end. This is a syntax error.
I receive the following error message: {{{ ^[[1mWARNING level 0 from host zwicky002 process 0 while executing schedule bin (none), routine (no thorn)::(no routine) in thorn cactus, file mk-mclachlan-funhpc.par:1: ->^[[0m ERROR IN PARAMETER FILE:In rule 'file::set' Line=100, Column=95 # ML_BSSN_FH::block_size_i = 4 # ML_BSSN_FH::block_size_j = 4 # ML_BSSN_FH::block_size_k = 4
ML_BSSN_FH::ML_log_confac_bound = "none" ^ Expected one of the following characters: [[ \t\r\n#=] }}}
This error message is unclear because - it doesn't show the line that has the error; instead, it show only the lines that follow (and that are correct) - the set of "following characters" is correct, but I got confused by all the white space characters that are allowed; a description "expected [ or =" might have been more clear - the actual error is that there is a parameter, but this parameter is not followed by a value; this is not described, since the error message has a rather low level ("next character") instead of a high level ("value missing")
#1881: Unclear error message for parameter file error -----------------------+---------------------------------------------------- Reporter: eschnett | Owner: Type: defect | Status: new Priority: minor | Milestone: Component: Cactus | Version: development version Resolution: | Keywords: -----------------------+----------------------------------------------------
Comment (by sbrandt):
I have a fix. Let's see if this agrees with what you expect. First, here's what I used for a par file. {{{ # ML_BSSN_FH::block_size_i = 4 # ML_BSSN_FH::block_size_j = 4 # ML_BSSN_FH::block_size_k = 4
ML_BSSN_FH::ML_log_confac_bound = "none" ML_BSSN_FH_Helper::ML_BSSN_FH_CalculateRHSInMoLPostStep }}} Here is the error I now produce: {{{ WARNING level 0 from host sbrand3-5.lsu.edu process 0 while executing schedule bin (none), routine (no thorn)::(no routine) in thorn cactus, file x.par:1: -> ERROR IN PARAMETER FILE:Parse Error Expected one of the following characters: [[=]
# ML_BSSN_FH::block_size_k = 4
ML_BSSN_FH::ML_log_confac_bound = "none" ML_BSSN_FH_Helper::ML_BSSN_FH_CalculateRHSInMoLPostStep ^
}}} This shows the correct line and position in the line, and the set of characters that it expected. Is [[=] too cryptic?
#1881: Unclear error message for parameter file error -----------------------+---------------------------------------------------- Reporter: eschnett | Owner: Type: defect | Status: new Priority: minor | Milestone: Component: Cactus | Version: development version Resolution: | Keywords: -----------------------+----------------------------------------------------
Comment (by rhaas):
For all the "expected one of the following character" message from piraha: I would not include the surrounding pair of "[]" since a literal reading of the text would imply that "[" and "]" are allowed characters at this position. Ie the user should not need to know that regular expressions are at all involved in piraha (since this is an implementation details best hidden from the user).
#1881: Unclear error message for parameter file error -----------------------+---------------------------------------------------- Reporter: eschnett | Owner: Type: defect | Status: new Priority: minor | Milestone: Component: Cactus | Version: development version Resolution: | Keywords: -----------------------+----------------------------------------------------
Comment (by knarf):
Replying to [comment:2 rhaas]:
For all the "expected one of the following character" message from
piraha: I would not include the surrounding pair of "[]" since a literal reading of the text would imply that "[" and "]" are allowed characters at this position. Ie the user should not need to know that regular expressions are at all involved in piraha (since this is an implementation details best hidden from the user).
I agree in general, but the problem with that might be that these expressions aren't always as simple as a single [] construct, i.e., it is not always just a list of allowed (or not allowed) characters only.
#1881: Unclear error message for parameter file error -----------------------+---------------------------------------------------- Reporter: eschnett | Owner: Type: defect | Status: new Priority: minor | Milestone: Component: Cactus | Version: development version Resolution: | Keywords: -----------------------+----------------------------------------------------
Comment (by knarf):
Or in other words: I suspect Piraha "knows" at that moment only that some input didn't match a specific regular expression. It doesn't "know" that that particular regular expression might have been a quite simple one. We could add a check for that just for the sake of error messages, but would it be worth it? One also would need to remove the escape for "[", and possibly parse other stuff inside the regular expressions, e.g., a leading "^".
#1881: Unclear error message for parameter file error -----------------------+---------------------------------------------------- Reporter: eschnett | Owner: Type: defect | Status: review Priority: minor | Milestone: Component: Cactus | Version: development version Resolution: | Keywords: -----------------------+---------------------------------------------------- Changes (by knarf):
* status: new => review
Comment:
I propose to apply this change (as it improves things considerably), and discuss an even better, more general solution, during a call/meeting.
#1881: Unclear error message for parameter file error -----------------------+---------------------------------------------------- Reporter: eschnett | Owner: Type: defect | Status: review Priority: minor | Milestone: Component: Cactus | Version: development version Resolution: | Keywords: -----------------------+----------------------------------------------------
Comment (by sbrandt):
Patch updated with clearer error messages. Now you see this: {{{ WARNING level 0 from host sbrand3-5.lsu.edu process 0 while executing schedule bin (none), routine (no thorn)::(no routine) in thorn cactus, file x.par:1: -> ERROR IN PARAMETER FILE:Parse Error Expected one of the following characters: '[', '=' # ML_BSSN_FH::block_size_k = 4
ML_BSSN_FH::ML_log_confac_bound = "none" ML_BSSN_FH_Helper::ML_BSSN_FH_CalculateRHSInMoLPostStep ^
application called MPI_Abort(MPI_COMM_WORLD, 1) - process 0
}}} and if you add the = sign, you get this {{{ WARNING level 0 from host sbrand3-5.lsu.edu process 0 while executing schedule bin (none), routine (no thorn)::(no routine) in thorn cactus, file x.par:1: -> ERROR IN PARAMETER FILE:Parse Error Expected one of the following characters: '[', '+', '-', '!', '(', '0' to '9',double quote, '.', '/', 'a' to 'z', 'A' to 'Z', '$' # ML_BSSN_FH::block_size_k = 4
ML_BSSN_FH::ML_log_confac_bound = "none" ML_BSSN_FH_Helper::ML_BSSN_FH_CalculateRHSInMoLPostStep = ^
application called MPI_Abort(MPI_COMM_WORLD, 1) - process 0 }}}
#1881: Unclear error message for parameter file error -----------------------+---------------------------------------------------- Reporter: eschnett | Owner: Type: defect | Status: review Priority: minor | Milestone: Component: Cactus | Version: development version Resolution: | Keywords: -----------------------+----------------------------------------------------
Comment (by rhaas):
I like the example output that you provide. I am however loath to apply this very close to the release unless it is very clear it cannot make parsing abort. Unfortunately the patch introduces a rather large amount of what seems whitespace only changes making review quite hard. Would you be able to provide a patch with less whitespace changes so that it is easier to see what the "payload" changes are?
#1881: Unclear error message for parameter file error -----------------------+---------------------------------------------------- Reporter: eschnett | Owner: Type: defect | Status: review Priority: minor | Milestone: Component: Cactus | Version: development version Resolution: | Keywords: -----------------------+----------------------------------------------------
Comment (by sbrandt):
Whitespaces removed.
#1881: Unclear error message for parameter file error -----------------------+---------------------------------------------------- Reporter: eschnett | Owner: Type: defect | Status: review Priority: minor | Milestone: Component: Cactus | Version: development version Resolution: | Keywords: -----------------------+----------------------------------------------------
Comment (by rhaas):
hmm, this will take a while I am afraid. The code is still very hard for me to read since it introduces many new lines of code and changes other lines above and below as well. It also seems to be completely uncommented, using very short variable names to give me no hint as to what they may mean, making this a very hard to maintain code I fear.
One question I have even before digging any deeper: why does the human readable range only include 'a' to 'y' and not 'a' to 'z' (and similar for digits and uppercase letters)? Should the underscore '_' be included in human readable (this would make human readable the class of characters allowed for C identifiers).
#1881: Unclear error message for parameter file error -----------------------+---------------------------------------------------- Reporter: eschnett | Owner: Type: defect | Status: review Priority: minor | Milestone: Component: Cactus | Version: development version Resolution: | Keywords: -----------------------+----------------------------------------------------
Comment (by sbrandt):
Possibly it's a suboptimal method name. The idea is that if we have two characters, a and b, and a+1 == b, and isHumanReadableRange(a) is true, then it can display as a range, i.e. with a dash between the characters. Thus, z, Z, 9, and _ should not be included.
#1881: Unclear error message for parameter file error -----------------------+---------------------------------------------------- Reporter: eschnett | Owner: sbrandt Type: defect | Status: accepted Priority: minor | Milestone: Component: Cactus | Version: development version Resolution: | Keywords: -----------------------+---------------------------------------------------- Changes (by sbrandt):
* owner: => sbrandt * status: review => accepted
trac@lists.einsteintoolkit.org
-
Einstein Toolkit