Skip to content

WarpX: Python on pyAMReX#45251

Merged
eugeneswalker merged 2 commits intospack:developfrom
ax3l:warpx-pyamrex
Aug 19, 2024
Merged

WarpX: Python on pyAMReX#45251
eugeneswalker merged 2 commits intospack:developfrom
ax3l:warpx-pyamrex

Conversation

@ax3l
Copy link
Copy Markdown
Member

@ax3l ax3l commented Jul 16, 2024 •
edited
Loading

Long overdue update for WarpX: in 2024, we updated our Python bindings to rely on the new pyAMReX package. This deprecates the old py-warpx package and adds a new dependency and variant to WarpX. This update also removes any vendored (auto-downloaded) packages from our superbuilds (i.e., AMReX) and properly uses external dependencies now.

Add @EZoni to WarpX package co-maintainers.

Also deprecates old versions that we will not continue to support.

@ax3l ax3l added the e4s label Jul 16, 2024
@ax3l
Copy link
Copy Markdown
Member Author

ax3l commented Jul 16, 2024

@spack/e4s for the WarpX E4S Packages, we can remove the py-warpx packages now but should add the +python variant to the deployed packages. +python is non-default in Spack packages by convention and thus default=False in the warpx package, too.

@ax3l ax3l force-pushed the warpx-pyamrex branch 2 times, most recently from 7c8f919 to e1f5bb1 Compare July 16, 2024 17:41
@eugeneswalker
Copy link
Copy Markdown
Contributor

eugeneswalker commented Jul 16, 2024

@spack/e4s for the WarpX E4S Packages, we can remove the py-warpx packages now but should add the +python variant to the deployed packages.

I'm confused. We should remove py-warpx but also add +python to the deployed packages??

@ax3l
Copy link
Copy Markdown
Member Author

ax3l commented Jul 16, 2024 •
edited
Loading

Correct. We can remove py-warpx but need to add +python to the variants of the deployed warpx package.

@eugeneswalker
Copy link
Copy Markdown
Contributor

eugeneswalker commented Jul 16, 2024

Correct. We can remove py-warpx but need to add +python to the variants of the deployed warpx package.

OK, so after removing py-warpx just use a packages:warpx:require:+python and that should do it-- does that sound right?

@ax3l ax3l force-pushed the warpx-pyamrex branch 4 times, most recently from f757b5a to 4c7ea54 Compare July 17, 2024 22:56
ax3l
ax3l commented Jul 17, 2024
View reviewed changes
@ax3l ax3l force-pushed the warpx-pyamrex branch 2 times, most recently from 86d226d to 32ed123 Compare July 17, 2024 23:55
@ax3l
Copy link
Copy Markdown
Member Author

ax3l commented Jul 17, 2024

With the latest patches, spack install -v warpx@develop +python already builds.

@ax3l ax3l mentioned this pull request Jul 18, 2024
Merged
@ax3l ax3l requested a review from eugeneswalker August 15, 2024 06:24
@ax3l
Copy link
Copy Markdown
Member Author

ax3l commented Aug 15, 2024

@eugeneswalker should I update the E4S changes we discussed above in the share/spack/gitlab/cloud_pipelines/ files of this repo with this PR? Or are the E4S recipes maintained in an external repo?

@eugeneswalker
Copy link
Copy Markdown
Contributor

eugeneswalker commented Aug 15, 2024

@eugeneswalker should I update the E4S changes we discussed above in the share/spack/gitlab/cloud_pipelines/ files of this repo with this PR? Or are the E4S recipes maintained in an external repo?

Yes, will you please update them here? We have recipes elsewhere and I can take care of those based on the changes you make to the pipelines in Spack repo.

@spackbot-app spackbot-app bot added core PR affects Spack core functionality gitlab Issues related to gitlab integration labels Aug 15, 2024
@ax3l
Copy link
Copy Markdown
Member Author

ax3l commented Aug 15, 2024

Thanks for the guidance, pushed now.

@ax3l
WarpX: Python on pyAMReX
01ea687 
Long overdue update for WarpX: in 2024, we updated our Python
bindings to rely on the new pyAMReX package. This deprecates the old
`py-warpx` package and adds a new dependency and variant to WarpX.

Also deprecates old versions that we will not continue to support.
@ax3l ax3l force-pushed the warpx-pyamrex branch 3 times, most recently from 088f473 to d230b16 Compare August 18, 2024 19:15
@ax3l
Copy link
Copy Markdown
Member Author

ax3l commented Aug 18, 2024

@eugeneswalker @tgamblin do you know if the compilers in the oneAPI CI runners for E4S are properly set up (e.g., via source /opt/intel/oneapi/setvars.sh)? I think the linker might not properly set because I observe that CMake's IPO tests fail.

In my mainline repo of pyAMReX, I build with oneAPI using LTO/IPO for python bindings, which is a practice highly encouraged by our dependency pybind11 due to the way it generates code. (I do not care much about general IPO support beyond that and understand that Spack defaults to off for this.)

I disabled IPO/LTO for py-amrex for now on the oneAPI runners to make the tests pass, but I think this can be fixed in Spack because I don't have the same issue with the oneAPI build chain outside of Spack.

@ax3l ax3l force-pushed the warpx-pyamrex branch 2 times, most recently from b113364 to 3fe6195 Compare August 18, 2024 20:35
@ax3l ax3l mentioned this pull request Aug 18, 2024
Closed
@ax3l ax3l force-pushed the warpx-pyamrex branch 2 times, most recently from bdee1fd to 27ecb87 Compare August 18, 2024 22:46
@spackbot-app spackbot-app bot added the patch label Aug 18, 2024
@ax3l
Update Cloud/E4S Pipelines for WarpX
a57eec6 
`py-warpx` is replaced by `warpx +python`
oneAPI does not support IPO/LTO: diable for `py-amrex` even though
pybind11 strongly encourages it.
@ax3l
Copy link
Copy Markdown
Member Author

ax3l commented Aug 19, 2024

Ready to merge 🎉

@eugeneswalker eugeneswalker merged commit c1852e3 into spack:develop Aug 19, 2024
@ax3l ax3l deleted the warpx-pyamrex branch August 19, 2024 04:14
@ax3l ax3l mentioned this pull request Aug 19, 2024
Closed
3 tasks
tldahlgren pushed a commit to AcriusWinter/spack that referenced this pull request Aug 20, 2024
@ax3l @tldahlgren
WarpX: Python on pyAMReX (spack#45251)
dbbf82f 
* WarpX: Python on pyAMReX

Long overdue update for WarpX: in 2024, we updated our Python
bindings to rely on the new pyAMReX package. This deprecates the old
`py-warpx` package and adds a new dependency and variant to WarpX.

Also deprecates old versions that we will not continue to support.

* Update Cloud/E4S Pipelines for WarpX

`py-warpx` is replaced by `warpx +python`
oneAPI does not support IPO/LTO: diable for `py-amrex` even though
pybind11 strongly encourages it.
@greenc-FNAL greenc-FNAL mentioned this pull request Aug 22, 2024
Merged
FrederickDeny pushed a commit to FrederickDeny/spack that referenced this pull request Aug 26, 2024
@ax3l @FrederickDeny
WarpX: Python on pyAMReX (spack#45251)
e2fb1fa 
* WarpX: Python on pyAMReX

Long overdue update for WarpX: in 2024, we updated our Python
bindings to rely on the new pyAMReX package. This deprecates the old
`py-warpx` package and adds a new dependency and variant to WarpX.

Also deprecates old versions that we will not continue to support.

* Update Cloud/E4S Pipelines for WarpX

`py-warpx` is replaced by `warpx +python`
oneAPI does not support IPO/LTO: diable for `py-amrex` even though
pybind11 strongly encourages it.
@jwillenbring jwillenbring mentioned this pull request Mar 19, 2025
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@eugeneswalker eugeneswalker eugeneswalker approved these changes

@dpgrote dpgrote Awaiting requested review from dpgrote

@MaxThevenet MaxThevenet Awaiting requested review from MaxThevenet

@RemiLehe RemiLehe Awaiting requested review from RemiLehe

@RTSandberg RTSandberg Awaiting requested review from RTSandberg

@sayerhs sayerhs Awaiting requested review from sayerhs

@WeiqunZhang WeiqunZhang Awaiting requested review from WeiqunZhang

@adamjstewart adamjstewart Awaiting requested review from adamjstewart

Assignees

No one assigned

Labels

conflicts core PR affects Spack core functionality dependencies e4s extends gitlab Issues related to gitlab integration new-variant new-version patch python update-package

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@ax3l @eugeneswalker @adamjstewart