Problems with expansions in parameter files

Comments (20)

1. 

   Frank Löffler

   • removed comment

   I thought in previous discussions we said, that the order in this kind of expressions should matter (not that I necessarily agreed - both has advantages and disadvantages). Wasn't this so?

   I agree to the error that should be raised though.

   • 2014-08-14T07:37:26+00:00
2. 

   Ian Hinder reporter

   • removed comment

   I remember a discussion about this issue on a telecon, but I don't remember a conclusion. Order-dependence is the sort of thing that leads to hard-to-find bugs

   • 2014-08-14T07:53:44+00:00
3. 

   Roland Haas

   • removed comment

   Technically the CoordBase::dy already has a value in the first line, namely the default value (which is 1). I am fine with making parameter files declarative rather than imperative. The required changes would seem to be:

   • have the parser check before using a variable value that it has been set (ie that the Parameter_NSet or whatever call returns >= 1)
   • forbid $a = 42 type assignments to happen more than once for the same variable (currently $a type assignments can be changed, see )

   The only legitimate use for order dependency would be something like this:

   $default_dx = CoordBase::dx
   CoordBase::dx = 42
   foo::bar = $default_dx
   

   or:

   $foo = 12
   bar::baza[0] = $foo
   bar::bazb[0] = $foo
   $foo = 13
   bar::baza[1] = $foo
   bar::bazb[1] = $foo
   

   ie to either safe the default value or to set up a set of similar stanzas. Both can be achieved without though by either spelling out the default from the param.ccl files or using more variables. I tend to think that the safety benefit of declarative parfiles outweights the usefulness of these corner cases.

   • 2014-08-14T09:22:24+00:00
4. 

   Steven R. Brandt

   • changed status to open
   • assigned issue to
   • removed comment

   • 2014-08-14T09:29:29+00:00
5. 

   Steven R. Brandt

   • removed comment

   The way Piraha currently does things is that it asks Cactus for the value of the parameter, which presumably comes from some default. That doesn't prevent you from setting it a new value later and using that.

   The attached patch will flag an error if a variable is written after it's read. That might give you part of what you're looking for.

   • 2014-08-14T14:39:54+00:00
6. 

   Steven R. Brandt

   • removed comment

   This patch changes the parameter file so that variables can only be set once. Cyclic assignments and multiple assignments are now errors. This change will temporarily break the TestPar test, which violates the new rules.

   In addition, this patch fixes a serious and previously undiscovered bug which caused unary operations to change the contents of the parameter they were applied to.

   • 2014-09-03T10:04:58+00:00
7. 

   Roland Haas

   • removed comment

   I am fairly sure do not understand setonce.patch. I am not sure I understand in which respect setting a variable only once is different from disallowing parameters to change after they are being read. setonce.patch seems much more complex that write_after_read.

   If both achieve the same effect (up to the unary-ops-change-their-argument issue), I'd prefer write_after_read.patch. It seems to me as if cyclical references can be detected if a parameter is marked as being "written" right when one begins to modify it and before the RHS of the assignment is actually evaluated.

   • 2014-09-08T10:26:52+00:00
8. 

   Steven R. Brandt

   • removed comment

   The first patch does not allow you to define parameters in arbitrary order, which is what Ian wanted.

   • 2014-09-08T11:09:19+00:00
9. 

   Roland Haas

   • removed comment

   For my money, I would actually prefer if parameters need to be defined before they are used, ie. I would prefer if

   a::c = a::b
   a::b = 12
   

   would throw an error just the same way that

   a::b = 12
   a::b = 13
   

   throws an error. If any order is allowed then I always have to read the full parfile instead of only the top part (or hopefully just around where the reference occurs since the original assignment happened close by).

   In fact

   a::c = a::b
   

   should assign to a::c the default value of a::b as far as I am concerned. Parameters always have a value in Cactus, even when just newly created by the flesh. :-)

   So I guess my preference would be for set-only-once rather than declarative and completely order independent.

   • 2014-09-08T11:43:43+00:00
10. 

    Ian Hinder reporter

    • removed comment

    It might be that what Roland is suggesting is better, but I would point out that requiring "set before use" might impose unnecessary structure on your parameter file. For example, you may want to put parameters into logical sections ("initial data", "evolution", "grid structure") etc, and having the requirement of "set before use" might lead to circular dependencies. I don't have an example, and I don't know if this would happen in practice, and it might indeed be easier to debug if we do it Roland's way, but I just wanted to mention this aspect here. I would be happy with a version which required "set before use" initially, and if we find it would be useful to relax the requirement, it could be changed later, without loss of backward compatibility.

    • 2014-09-09T06:42:33+00:00
11. 

    Erik Schnetter

    • removed comment

    This is connected to activating thorns as well. I typically like to activate thorns near the section where I set their parameters. Do we allow activating thorns in any order, or do we require them to be activated in order of dependency? We should do the same for parameters -- if thorns can be activated in any order, then we should allow parameters to be set in any order as well.

    • 2014-09-09T10:07:53+00:00
12. 

    Ian Hinder reporter

    • removed comment

    I dislike having to activate thorns in order, probably for the same reason that I dislike having to set parameters in order. I think of a parameter file as a description of a simulation, not an ordered list of assignment statements.

    • 2014-09-09T10:30:55+00:00
13. 

    Frank Löffler

    • removed comment

    I agree with Ian concerning thorns (dislike the current enforced order), and are "ok" with the same for parameters (although there I could see how a more complex solution might be nice - but then you can always use something Cactus-external (rpar).

    The current enforcement for thorns effectively leads to a bunch of "unordered", ugly, long thorn activation lines at the beginning of par files. I would like to move them around, like Ian. I don't have a strong opinion about variables on the other hand, but I didn't really use them outside of "rpar"s. It would be confusing to have the "declaration" further "down" the file than it's usage, but it could also be useful. We could leave the decision to use or not use this feature simply to the user.

    • 2014-09-09T12:51:51+00:00
14. 

    Roland Haas

    • removed comment

    I think allowing any order for parameter has a very similar effect as allowing parameters to be set more than once. In both cases one has to read the whole of the parfile to find out what value a given parameter has in the simulation. I would like to avoid this so that one can use a "find" in an editor to search for a parameter setting.

    I rather like structured files :-).

    • 2014-09-09T13:08:57+00:00
15. 

    Steven R. Brandt

    • removed comment

    What I'm sensing is that my original proposal, with the write-after-read error, is the preferred solution. This surprises me, as I had the impression from the call in which this issue was first discussed that the order independent solution was preferred.

    Thorn activation order no longer matters, BTW.

    • 2014-09-09T13:59:39+00:00
16. 

    Ian Hinder reporter

    • removed comment

    Replying to [comment:14 rhaas]:

    I think allowing any order for parameter has a very similar effect as allowing parameters to be set more than once. In both cases one has to read the whole of the parfile to find out what value a given parameter has in the simulation. I would like to avoid this so that one can use a "find" in an editor to search for a parameter setting.

    I don't follow. If a parameter can be set only once, it will appear only once in a parameter file. Thus, you can use "find" to search for it, and the first (and only) occurrence will be the correct one. This is independent of whether you allow parameter expressions to contain only parameters which have been set above them in the file. Did you mean something else? Were you referring to the ability to set parameters more than once, or have them use the default value if they had not been set "yet"?

    Here is another hint that we should allow any order. Suppose I have C = A followed by A = B. This would be disallowed currently, as the setting for A appears below the expression containing it. If I then remove the setting for A, magically the setting for C is now valid, as it will use the default value for A. In general, you cannot assume that you can find the value of a parameter by searching upward in the file, as the value might come from the defaults. Unless the setting is close to the use, you will probably have to search anyway, and then I don't think it makes a difference if the setting is above or below the use.

    • 2014-09-09T15:15:49+00:00
17. 

    Roland Haas

    • removed comment

    I agree that complete order independence lets me use an editors 'find' function. My example was not a good one.

    I still don't like usage patterns like this though:

    C = A
    ...lots of other stuff...
    A = B
    

    Mostly for two reasons: 1. Cactus parameters always have values, so C = A is valid even without A = B anywhere in the file. So, to me, having to read the whole file (rather than only the part above) to find out which of the two possible values of A is going to be used is unusual. Basically since all parameters in Cactus start out having values, parfiles always '''change''' parameters rather than '''set''' them. Order independence would seem to make more sense if all assignments are required. 1. I am very commonly asked to review parameter files that are about to run (or to debug them). I start reading them from the bottom and continue to the end. If the order is completely free then for each assignment I have to search to the bottom of the file to find the possible value of the parameter. If parameters cannot be set after they have been read, I can rely on having seen any such change in the part of the file that I have just read.

    Ian's example also contains magic in the case of order independence. In that case both

    C=A
    A=B
    

    and

    C=A
    

    are valid and also C would change its value from 'B' in the first case to the default of 'A' in the second. It's possible to remove the magic by requiring and "read" variable to have been "set" before. On the technical side, this needs to be an option for the expression parser so that parameters can be used in expressions that are not in parfile line righ-hand-sides (eg for the Trigger thorn or in accumulators).

    However it seems to me that both Ian and I have stated out preferences, which differ and that it may be best to get some second opinion on the matter.

    • 2014-09-11T06:41:15+00:00
18. 

    Frank Löffler

    • removed comment

    For what it is worth, most examples brought up use generic variable names 'A', 'B', ect. - while in practice a reader would most often "correctly guess" from the variable name what is done here:

    CarpetRegrid2::radius_1[1] = CoordBase::xmax / 2
    CarpetRegrid2::radius_1[1] = CarpetRegrid2::radius_1[2] / 2
    

    strongly suggests that CoordBase::xmax is set somewhere in the file, simply because that is something you would expect a user to always set. I am not arguing that it would not be possible to construct confusing examples - but rather that most cases would "intuitively understood" by the context.

    I would suggest to leave the position of an assignment free, but to provide a facility in Cactus that, given a parameter file, does not run past the parameter file parser (especially does not allocate large memory), and simply outputs the parameter file with all expressions replaced. This way a review of a parameter file would be made much simpler - there would be no surprises and replacements can easily be checked even by the person sending the parameter file for review, creating even less work for the reviewer. For convenience it might even provide the "diff" between the original and processed parameter file as well.

    • 2014-09-11T10:24:26+00:00
19. 

    Steven R. Brandt

    • changed status to resolved
    • removed comment

    Write after read patched applied in revision 5129

    • 2014-09-16T14:18:47+00:00
20. 

    Roland Haas

    • edited description
    • changed status to closed

    • 2019-02-21T20:05:31+00:00
21. Log in to comment