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

Context variables referencing variant values and other context vars #1306

Open
minrk opened this issue Jan 7, 2025 · 5 comments
Open

Context variables referencing variant values and other context vars #1306

minrk opened this issue Jan 7, 2025 · 5 comments

Comments

@minrk
Copy link
Contributor

minrk commented Jan 7, 2025

I'm looking at updating the fairly complex petsc recipe, and one thing that's been super useful for simplifying the recipe is defining variables that reference variant/other context variables. I'll need this less in a v1 recipe with the nicer conditionals, but it would still be useful.

Is there any way to define a variable that references another variable in a v1 recipe?

For example, the build string prefix is cuda${{ cuda_major }}_mpi${{ mpi }}_ if cuda is used, and just mpi${{ mpi }}_ if not. This string needs to be computed more than once, because it also goes in run_exports. Further, cuda_major is already a derivative value, since cuda_compiler_version contains a minor version that we need to exclude.

If this worked:

context:
    cuda_major: ${{ cuda_compiler_version | split('.') | first }}
    build_prefix:
        if: cuda_compiler_version != "None"
        then: cuda${{ cuda_major }}_mpi${{ mpi }}
        else: mpi${{ mpi }}

I think I could get pretty far. That would ideally mean templates in context vars where:

  1. context vars can be templates
  2. templates can reference variant values
  3. templates can reference context vars that are defined before them

I know I can recompute the whole thing every time it is used, but it is really a lot simpler and results in a clearer, more reliable, more likely correct recipe to compute these variables just once.

Is there any way to create variables that are derived from other context variables or variant values?

@wolfv
Copy link
Member

wolfv commented Jan 7, 2025

Context variables are parsed from top-to-bottom, and you can reference earlier context variables in the later ones).

I am, however, not sure right now wether the variant variables are already available in the context. I would need to check this myself. Would be good to add a test to confirm this and make sure that we do not regress on it.

@minrk
Copy link
Contributor Author

minrk commented Jan 7, 2025

apologies, I had trouble working on this, but it seems this is already the behavior! I was misinterpreting errors.

I got tripped up because

cuda_major: ${{ cuda_compiler_version | split('.') | first }}

doesn't work because cuda_compiler_version from the variant is the number 11.8 not the string '11.8', so I got:

cuda_major: "${{ cuda_compiler_version | split('.') | first }}"
    ·               ─────────────────────────┬─────────────────────────
    ·                                        ╰── invalid operation: value is not a string

(would be nice if it said "11.8 is a number not a string", rather than just value)

whereas casting to string with lower (there's no str?) appears to do exactly what I want:

  cuda_major: "${{ cuda_compiler_version | lower | split('.') | first }}"

@minrk
Copy link
Contributor Author

minrk commented Jan 7, 2025

@wolfv thanks for commenting! Once I fixed the casting to a string, it really is exactly what I wanted (great job!). So variants are available and context vars can reference both earlier context vars and variants. A context var can even reference a variant value of the same name, so you can have default values overridden by the variant if defined. So this works:

context:
    maybe_variant: ${{ maybe_variant | default("variant_undefined") }}
    needs_variant: ${{ variant_value }}
    combined: ${{ maybe_variant }}_${{ needs_variant }}

all seems to resolve as I'd expect with and without variants. It would be awesome for this to be tested and documented, but I have no feature requests here, I think.

@wolfv
Copy link
Member

wolfv commented Jan 8, 2025

That's great to hear. Although it does sound as if there is a bug lingering becuase 11.8 should never be treated as a number, and especially floats should not really have a place in the recipes. I need to double check where that comes from because IMO it should always be a string.

@minrk
Copy link
Contributor Author

minrk commented Jan 8, 2025

Yeah, it is a string if I define it in context, but seems to be a number when coming from the variant file, if that helps.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants