legion: add new releases, add realm as new dependency - #2997
Conversation
36f6df1 to
aae6a74
Compare
tldahlgren
left a comment
There was a problem hiding this comment.
Confirmed the new version commit+tags.
@elliottslaughter @pmccormick @streichler Any objections to this PR being merged?
Spoke too soon. Looks like there are CI failures with the package, e.g., https://gitlab.spack.io/spack/spack-packages/-/jobs/20470139. |
|
My main feedback is that the the Realm version is unpinned. The only version that the Legion developers are willing to promise to work with is the one specifically pinned in the Legion repository. As of Legion 25.09.0 and 25.12.0 that version is StanfordLegion/realm@7ef8789. The version in current master is StanfordLegion/realm@0ef7edc and I'd expect that before our next release we'll probably bump it at least once more. Other versions of Realm may work but you're on your own in terms of ensuring compatibility. |
|
Echoing @elliottslaughter above, is there some way to add default Realm versions for Legion releases? It's not clear to me how to do that in Spack when the Realm version included by Legion doesn't really have a version number associated with it (as far as Spack can tell). To use this package currently, I check out the Legion source tree in a local "develop" tree, and symlink the Realm source in that tree to create what looks to Spack like a Realm develop tree. I haven't encountered any problems using that method, but it's pretty clumsy. |
|
I have a proposal for your consideration. I'm going to assume that the Realm package eventually gets versions beyond "main", but this proposal doesn't require it. For argument's sake, assume that Realm has versions 1.0 and 1.1, as both inherent Realm versions and Realm Spack package versions. Now assume that Legion version 2028.03.0 uses a git commit somewhere between Realm 1.0 and 1.1, say with a commit sha of "deadbeef". My proposal starts with creating a Spack package version for Realm like version("1.0-lg2028.03.0", commit="deadbeef")I believe that adding the "-lg2028..." suffix to the version number puts that version between 1.0 and 1.1 according to Spack version number ordering. Additionally, if Realm does not add version numbers but may eventually do so, I believe that a standalone "lg2028..." version number would precede any numeric version number as well as "main". We could of course do this in Spack without adding tags to Realm's repository. How much the version ordering matters really depends on how we might use those numbers in the Legion package, as I show below. Next, for the Legion package, we add a boolean variant, say variant("safe_realm", default=True, description="Restrict Realm dependency to the known working version")
depends_on("realm") # option A
depends_on("realm@1.0-lg2028.03.0:", when="@2028.03.0 ~safe_realm") # option B
depends_on("realm@1.0-lg2028.03.0", when="@2028.03.0 +safe_realm") # always include thisWhich option is favored may depend on whether there is (or will be) a total ordering of the Realm package version numbers, including the versions used by Legion. Of course, that pattern would have to be extended for different Legion package versions. My motivation is to make it easier for Legion users to install a specific version of Legion with the approved Realm version for the release. I would guess this is the primary, or even sole, use case for the Realm package, as I know of no other Soack packages that depend on Realm. It is important to make this use case work reliably. While there may be other use cases for a Realm dependency, and I think this proposal can support them, given the current situation "fixing" those (speculative) cases is less urgent. Comments? |
|
All the above aside, this package is missing the propagation of |
|
@elliottslaughter to make any meaningful solution, similar to what @mpokorny proposes, it would be good to know what versioning scheme Realm will be targeting. Is it going to follow the |
|
@mpokorny Added a few updates. This now adds two realm versions that match what is bundled in legion. It's not perfect, but allows the maximum flexibility while providing meaningful defaults.
The main question here is whether the selected versioning scheme in Realm matches what they will use in the end. Otherwise we would have to fix these versions later on to not interfere with the regular version selection logic. |
|
Looks like they followed Hopefully we'll get actual Realm releases at some point, but until then this seems reasonable to me. |
mpokorny
left a comment
There was a problem hiding this comment.
I like what you've done, with one correction.
I agree that we should understand the version numbering Realm intends to use, if any.
- add v25.09.0, v25.12.0 - limit cuda@:12.8 restriction up to 24.12.0 - ignore error for clang@20+ - uses gasnet Spack package as dependency, use external for complex configurations
|
@elliottslaughter With #1409 merged, we now have usable gasnet spack package. Can we proceed with merging this one? Realm doesn't force any particular gasnet configuration based on the conduit selection like legion did, but this is a good start. |
|
Looks good to me, thanks. |
* legion: add realm as new dependency - add v25.09.0, v25.12.0 - limit cuda@:12.8 restriction up to 24.12.0 - ignore error for clang@20+ - uses gasnet Spack package as dependency, use external for complex configurations * legion: propagate max_dims to realm * legion: add v26.03.0 and 'safe' realm versions * e4s: don't try building legion without cuda_arch
Extracted from #1231 without explicit changes to the GASNet Spack package. Based on #2941.