Re: [Condor-devel] RFC: A corner case in classad scoping for select/subscript operator

[Erik Erlandson <eje@xxxxxxxxxx>]

On Thu, 2012-07-12 at 14:53 -0500, Dan Bradley wrote:
> Hi Erik,
> 
> The behavior you describe sounds wonky to me.  I would expect an 
> implicitly scoped reference to AccountingGroup from the job ad to point 
> to the job ad attribute, not the machine ad.  I would expect this 
> irrespective of whether the reference is being evaluated directly in the 
> job ad or indirectly via a reference from the machine ad to the 
> reference in the job ad.

I just realized the internets ate my 'here' link:
http://tinyurl.com/min-max-resources

An even simpler example might be easier to discuss.  Consider a
simplified version of the constructed 'match ad':

[
    left = [
        x = 2;
        le = x;
    ];
    right = [
        x = 0;
        requirements = (parent.left.le > 1);
    ];
]

What should right.requirements evaluate to?  Is the correct answer
'true' because 'le' evaluates in scope "left" where x=2, or is the
correct answer 'false' because le evaluates in scope "right" where x=0?

If the answer is supposed to be 'true' (which is how I've always assumed
it worked) then I think I have turned over a bug, because the behavior I
was seeing was equivalent to evaluating where x=0 and getting 'false'

Regardless of what the answer should be, the classad spec document
should be fixed to give the context argument that represents the right
answer:

(E', C') = eval(lookup(s,(Eb',Cb')), need_context_here)

(we might want to add an example or two as well, since that portion of
the spec is a bit terse and hard to unpack)


> 
> The fact that the behavior changes when you add explicit "my" scope 
> seems unexpected too.
> 
> This sounds like a goof to me.  I am responsible for having optimized 
> the matchClassAd.  I hope I didn't somehow mess this up!
> 
> --Dan
> 
> On 7/12/12 1:34 PM, Erik Erlandson wrote:
> > While I was constructing an example configuration for min/max resources,
> > I created a fairly confusing situation where I indirectly referenced an
> > attribute (in fact, two: AccountingGroup and ConcurrencyLimits) which
> > appear in both the machine ad and the job ad.
> >
> > There's a discussion of the specific configuration here. In the
> > abstract, I had something like this:
> >
> > machine ad:
> > [ ...
> > requirements = e1(other.AG)
> > ... ]
> >
> > where e1 is an expression including the attribute name "AG".  Now, "AG"
> > is defined on the *job* ad, like so:
> >
> > job ad:
> > [...
> > AG = e2(AccountingGroup)  # an expression involving the attr name
> > "AccountingGroup"
> > ...]
> >
> >
> > When it came time to test a match by evaluating the requirements, it
> > evaluated:
> >
> > eval(e1(e2(AccountingGroup)), machine_ad)
> >
> > that is, it deferred evaluation of e2(AccountingGroup), and then used
> > the value of AccountingGroup scoped by the machine ad, where what I
> > really wanted (and expected) was AccountingGroup scoped to the job ad.
> >
> > What I did to fix it was make scope more explicit(*) on the job ad side:
> >
> > job ad:
> > [...
> > AG = e2(my.AccountingGroup)  # scopes AccountingGroup to job ad
> > ...]
> >
> > I'm curious about whether this deferred evaluation, under the machine ad
> > scope, is a correct interpretation of the classad language spec (Tim's
> > assessment is "yes it is", and he is quite possibly right).  The
> > alternative interpretation would be that 'other.AG' evaluates as:
> > eval(e2(AccountingGroup), job_ad) - that is, the RHS is evaluated in the
> > scope of the base "other".
> >
> > I consulted the spec:
> > http://research.cs.wisc.edu/condor/classad/refman/node4.html
> >
> > and strangely enough, the spec itself literally is missing that
> > information on scoping: the nut of the definition is here:
> >
> > "If Es' is a String with value s and Eb' is a Record expression, then
> > (E', C') = eval(lookup(s,(Eb',Cb')))"
> >
> > But the eval() function requires a scope as its second argument, and
> > that 2nd arg is missing in the definition, which happens to be exactly
> > the thing that clarifies what the scope should be according to the spec!
> >
> > I'm interested in any community insights into what that 2nd scoping arg
> > was intended to be.
> >
> >
> > (*) Actually, if the expected behavior is:
> >          eval(e1(e2(my.AccountingGroup)), machine_ad)
> >
> > then arguably a user shouldn't expect that using "my." would fix it
> > either, since one might expect "my." to then refer to machine_ad.
> >
> > In practice "my." is pre-substituted and that is why it works, but it
> > seems to conflict with the eval logic where evaluation of other.AG -->
> > eval(e1(e2(AccountingGroup)), machine_ad).  Although it allowed me to
> > get what I needed, it seems that if the one didn't work, this arguably
> > shouldn't work either.
> >
> > That is, presubstituting "my.", "other.", and friends to ".left" and
> > ".right" is a condor thing, but not exactly a classad language spec
> > thing.  Apparently done to make eval go faster.  The construction that
> > is described in the header doc for matchClassAd.h (but not actually
> > built as described) is more aligned with the spec, although it would be
> > slower:
> >
> >      [
> >         symmetricMatch   = leftMatchesRight && rightMatchesLeft;
> >         leftMatchesRight = RIGHT.requirements;
> >         rightMatchesLeft = LEFT.requirements;
> >         leftRankValue    = LEFT.rank;
> >         rightRankValue   = RIGHT.rank;
> >         RIGHT            = rCtx.ad;
> >         LEFT             = lCtx.ad;
> >         lCtx             =
> >             [
> >                 other    = .RIGHT;
> >                 target   = .RIGHT;   // for condor backwards compatibility
> >                 my       = .LEFT;    // for condor backwards compatibility
> >                 ad       =
> >                    [
> >                        // the ``left'' match candidate goes here
> >                    ]
> >      	   ];
> >         rCtx             =
> >             [
> >                 other    = .LEFT;
> >                 target   = .LEFT;    // for condor backwards compatibility
> >                 my       = .RIGHT;   // for condor backwards compatibility
> >                 ad       =
> >                    [
> >                        // the ``right'' match candidate goes here
> >                    ]
> >      	   ];
> >      ]
> >
> >
> >
> > _______________________________________________
> > Condor-devel mailing list
> > Condor-devel@xxxxxxxxxxx
> > https://lists.cs.wisc.edu/mailman/listinfo/condor-devel
> 
> 
> _______________________________________________
> Condor-devel mailing list
> Condor-devel@xxxxxxxxxxx
> https://lists.cs.wisc.edu/mailman/listinfo/condor-devel