Skip to content

fix the pyresample for intel issue - #94

Merged
climbfuji merged 11 commits into
JCSDA:spack-stack-devfrom
Hang-Lei-NOAA:spack-stack-dev
Oct 14, 2026
Merged

climbfuji merged 11 commits into
JCSDA:spack-stack-devfrom
Hang-Lei-NOAA:spack-stack-dev

Conversation

@Hang-Lei-NOAA

Copy link
Copy Markdown
Collaborator

For JCSDA/spack-stack#1732

(1) pyresample is added into spack spack#6333

(2) Fix the pyresample build issue with intel:
modify py_pyresample/package.py in spack-stack that adds depends_on("c", type="build") and depends_on("cxx", type="build")
modify py_pykdtree/package.py in spack-stack that adds depends_on("c", type="build") and fixes the OpenMP issue for %oneapi (patch -lgomp → -liomp5 or set USE_OMP=0)

@Hang-Lei-NOAA
Hang-Lei-NOAA marked this pull request as ready for review October 5, 2026 18:23
@climbfuji

climbfuji commented Oct 5, 2026 •

Copy link
Copy Markdown
Collaborator

Thanks @Hang-Lei-NOAA - have you tested these with Intel oneAPI (ifort or ifx)?

You can create a PR for spack-stack that temporarily points to your spack-packages branch if you want to use the CI runners for spack-stack for testing.

@Hang-Lei-NOAA Hang-Lei-NOAA mentioned this pull request Oct 8, 2026
4 of 9 tasks
@Hang-Lei-NOAA

Copy link
Copy Markdown
Collaborator Author

@climbfuji This has been tested within spack-stack (JCSDA/spack-stack#2140) with intel oneapi.

@climbfuji climbfuji left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Hang-Lei-NOAA @AlexanderRichert-NOAA I wonder if there's a cleaner way to patch the code, or not bother and build it with gcc (if it's a build dependency)?

There's no version specifier in the patch function. Does this really apply cleanly to all versions listed above?

Also, line 74 should probably become a with when decorator just before def patch if the patch stays this way.

Lastly, we need to send the changes we merge here upstream once they are final.

@Hang-Lei-NOAA

Copy link
Copy Markdown
Collaborator Author

1, The patch is the right approach. Forcing GCC isn't clean because py-numexpr is type=("build", "run") — constraining it to %gcc in a mostly-Intel environment creates a mixed-compiler stack that's worse. The patch is invasive but correct.

2, bytes.replace() is a silent no-op when the target string isn't present, so old versions won't break. Older versions may lack the fmaxd/fmind helper functions entirely (added around 2.6), in which case those replace calls just do nothing and there's no Intel build failure for those older code paths anyway. No version specifier needed on the method itself — the @when compiler guards are the right scope.

3, Great suggestion! Now split it into @when("%oneapi") and @when("%intel") decorators that both delegate to _patch_intel(). This is the canonical spack pattern when you need OR across two conditions: two decorators, one helper.

Comment thread repos/spack_repo/builtin/packages/py_numexpr/package.py Outdated

@climbfuji climbfuji left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The patch for py-numexpr is far too invasive and likely not acceptable upstream. What's more, we don't even need it.

First, I established that out of all py-numexpr versions defined here, version 2.10.2 builds with Intel oneAPI. Versions 2.9 and earlier can't be build with Intel oneAPI, because they depend on py-numpy@:1.25 which don't build with Intel oneAPI either.

Version 2.14.2 doesn't build, which is what you are trying to fix. That fix could be much cleaner and contained in a small .patch file.

However, none of this is needed. If you look inside the py-pyresample code, then py-numexpr is used only in one place in _spatial_mp.py, and there is a fallback to numpy if numexpr is not available. Further, numexpr is not constrained by pyresample, means we could use 2.10.2. In the py-resample build scripts, numexpr is only an optional dependency.

My recommendation is to remove the py-numpexpr dependency from py-resample and remove the updates to py-numexpr entirely from this PR.

@Hang-Lei-NOAA

Copy link
Copy Markdown
Collaborator Author

@climbfuji Thanks for your suggestion. I missed it util Alex remind me yesterday. Now, I have revised it according to it, and have it tested in JCSDA/spack-stack#2140

@climbfuji climbfuji left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, looks good to me

@AlexanderRichert-NOAA AlexanderRichert-NOAA left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Much cleaner! :)

@climbfuji
climbfuji disabled auto-merge October 14, 2026 17:16
@climbfuji
climbfuji merged commit c87baea into JCSDA:spack-stack-dev Oct 14, 2026
@github-project-automation github-project-automation Bot moved this from In Progress to Done in spack-stack-2.2.x (2026 Q?) Oct 14, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Development

Successfully merging this pull request may close these issues.

3 participants