Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Variant Forwarding Proposal #2594

Closed
citibeth opened this issue Dec 15, 2016 · 8 comments
Closed

Variant Forwarding Proposal #2594

citibeth opened this issue Dec 15, 2016 · 8 comments

Comments

@citibeth
Copy link
Member

Comments / criticisms appreciated. This is a first stab at it.

In this proposal, the Spack concretizer and package syntax will be upgraded to allow variants to be "forwarded" through the DAG. This can be thought of as, effectively, the Spack concretizer overwriting the DAG through the concretization process (sorry, I don't have a well thought out algorithm). Forwarding happesn before concretization.

In general... if A->B and we want to forward the x variant from A to B, then this would be accomplished by the following transformations:

A+x ==> A+x^B+x
A~x ==> A~x^B~x
A ==> A^B

In the last case, B will receive whatever value for x it "naturally" would have had without variant forwarding. It would be an error to forward x to B if B does not have variant x to begin with.

Variants can also be forwarded transitively. If A->B->C, and we "transitively forward" variant x from A to B, then C will also receive x --- even if B does not use the variant x.

Although it is possible in theory to forward any variant in A to any variant in B, this will not be permitted: variant forwarding must be between variants of the same name.

Variant forwarding can be specified in the package.py files. For example:

class A(Package):
    variant('x')
    depends_on('B', forward=('x',))     # Regular forwarding
    depends_on('C', forward=('x*',))   # Forward transitively

We can also forward variants from packages.yaml. For example:

    icebin:
        version: [develop]
        variants: +*gridgen+python~**everytrace

In this case, +*gridgen sets the gridgen variant and then forwards it to all dependencies; ~**everytrace clears the everytrace variant and then forwards it transitively to all dependencies.

@becker33
Copy link
Member

There was a previous PR from @alalazo that implemented some of these features, #391. It's been closed since, but we have some users at LLNL who have encorporated this feature already; it could be a valuable place to start thinking about this.

At the time @tgamblin and I had thought that we preferred a version that had variant forwarding as an attribute of the variant and not the dependency. I don't know what Todd thinks now, but I'm starting to come around to the view that you and Massimiliano have espoused with his PR and your proposal.

@davydden
Copy link
Member

davydden commented Dec 15, 2016

Variants can also be forwarded transitively. If A->B->C, and we "transitively forward" variant x from A to B, then C will also receive x --- even if B does not use the variant x.

In this case Spack conretizer should be clever enough to see that B does not support a variant, but still pass it along to C? However in your example it would not be possible to define because you attach forwarding to depends_on:

class A(Package):
    variant('x')
    depends_on('B', forward=('x*',))   # Forward transitively

class B(Package):
    ## variant('x')  ## <-- no variant
    depends_on('C', forward=('x*',))   # <--- this part does not look right:

class C(Package):
    variant('x')  

Thereby i think

At the time @tgamblin and I had thought that we preferred a version that had variant forwarding as an attribute of the variant and not the dependency

this would be more natural way to do it, even though a variant would be forwarded to all dependents.

I have not looked into @alalazo 's PR #391, but it would certainly make a lot of cense to first see what was his approach and the interface to define such variants.

@citibeth
Copy link
Member Author

citibeth commented Dec 15, 2016 via email

@davydden
Copy link
Member

davydden commented Dec 15, 2016

Once
A has decided to transitively forward a variant, all descendants of A (that
define x) will get it, regardless of any other forwarding instructions
further down the DAG.

I see, but that is more in line of what @becker33 suggests that forwarding is a property of a variant, not depends_on(). Going to your suggested interface example, I would change

class A(Package):
    variant('x')
    depends_on('B', forward=('x*',))   # Forward transitively

class B(Package):
    depends_on('C1')   # <--- both C1 and C2 will get x forwarded from 
    depends_on('C2')   # <--- A without any extra steps

class C(Package):
    variant('x')
  
class D(Package):
    variant('x')  

to

class A(Package):
    variant('x', forward='transitive')  # other values: 'no', 'yes'
    depends_on('B1')   # Forward transitively
    depends_on('B2')   # Forward transitively

class B1(Package):
    depends_on('C1')   # <--- both C1 and C2 will get x forwarded from 
    depends_on('C2')   # <--- A without any extra steps

class C1(Package):
    variant('x')
  
class C2(Package):
    variant('x')  

to make the whole forwarding a bit more predictable and easy to understand.

@citibeth
Copy link
Member Author

citibeth commented Dec 15, 2016 via email

@davydden
Copy link
Member

We should consider use cases to see if we need
that flexibility.

Good idea.

@citibeth citibeth changed the title Dependency Forwarding Proposal Variant Forwarding Proposal Dec 21, 2016
This was referenced Dec 21, 2016
@citibeth citibeth mentioned this issue Feb 10, 2017
@alalazo alalazo self-assigned this Nov 22, 2017
@shuds13
Copy link
Contributor

shuds13 commented Sep 17, 2020

@scheibelp @becker33 @balay

Following discussion with @scheibelp at the recent Spack telecon, I would like to register an interest in variant propagation from the perspective of the xSDK metapackage. I am also making an effort to establish some common variant names/types for xSDK packages.

I can see benefits to various suggestions, but I think my initially preferred use-case would be to have forwarding defined as an attribute of the variant (this could prevent a lot of replication), combined with packages ignoring the variant if they do not implement it. This attribute could then be used in our metapackage for variants in our common variant list. Non-transitive forwarding would be sufficient initially. However, I can also see scenarios where transitive forwarding would be an advantage, like building a complete software stack in debug mode. I am open to alternative suggestions on any of this.

I assume that any package that does not define the forwarded variant would simply ignore it. But I'm wondering what happens in the case of a multi-value variant, if the variant exists but the forwarded value is not a listed option. Would that also be ignored (perhaps with a message), or would that result in an error?

@alalazo
Copy link
Member

alalazo commented Mar 18, 2023

Forwarding variants has been added in v0.19, with a different syntax, from specs. See https://spack.readthedocs.io/en/latest/basic_usage.html#variants

@alalazo alalazo closed this as completed Mar 18, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants