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

AMReX/pyAMReX/PICSAR: weekly update #5391

Merged
merged 3 commits into from
Oct 21, 2024
Merged

Conversation

EZoni
Copy link
Member

@EZoni EZoni commented Oct 14, 2024

  • Weekly update to latest AMReX:
./Tools/Release/updateAMReX.py
  • Weekly update to latest pyAMReX:
./Tools/Release/updatepyAMReX.py
  • Weekly update to latest PICSAR (no changes):
./Tools/Release/updatePICSAR.py

@EZoni EZoni added the component: third party Changes in WarpX that reflect a change in a third-party library label Oct 14, 2024
@WeiqunZhang
Copy link
Member

Looks like WarpX's NamedComponentParticleContainer needs an update. AddRealComp(const std::string& name, ...) exists now in AMReX.

@EZoni
Copy link
Member Author

EZoni commented Oct 14, 2024

@atmyers

I renamed the WarpX functions AddIntComp and AddRealComp, defined in Source/Particles/NamedComponentParticleContainer.H, as NewIntComp and NewRealComp, respectively, in order to avoid confusion with the AMReX functions of the same name, defined in Src/Particle/AMReX_ParticleContainer.H.

Does this look good to you? There are only two instances left where we are directly calling the AMReX functions from WarpX, without wrapping them in WarpX functions.

It's not clear to me whether the goal of recent PRs such as AMReX-Codes/amrex#4163 was to remove the WarpX functions and use directly the AMReX ones.

@EZoni
Copy link
Member Author

EZoni commented Oct 14, 2024

@atmyers

Also, in WarpX's newly renamed functions NewIntComp and NewRealComp we call the AMReX functions AddIntComp and AddRealComp by passing only the boolean comm flag. I have not changed this in this PR. However, I'm wondering if we are supposed to be passing also the name strings when we call those AMReX functions.

@EZoni EZoni changed the title AMReX/pyAMReX/PICSAR: weekly update [WIP] AMReX/pyAMReX/PICSAR: weekly update Oct 15, 2024
@EZoni
Copy link
Member Author

EZoni commented Oct 15, 2024

Discussed on Slack:
@atmyers will open a PR to call the new AMReX functions and remove NamedComponentParticleContainer from WarpX.

@EZoni
Copy link
Member Author

EZoni commented Oct 21, 2024

Rebase after #5399 to pass the macOS / AppleClang check.

@EZoni EZoni self-assigned this Oct 21, 2024
@EZoni EZoni changed the title [WIP] AMReX/pyAMReX/PICSAR: weekly update AMReX/pyAMReX/PICSAR: weekly update Oct 21, 2024
@EZoni EZoni enabled auto-merge (squash) October 21, 2024 20:56
@EZoni EZoni mentioned this pull request Oct 21, 2024
@EZoni EZoni disabled auto-merge October 21, 2024 21:43
@EZoni
Copy link
Member Author

EZoni commented Oct 21, 2024

@dpgrote

I just disabled auto-merge here, please feel free to review and suggest further changes as in #5401 if you see that something is missing.

Here we decided to rename the WarpX functions, since we thought it creates less confusion with the AMReX functions, until the permanent change (which will remove the WarpX functions) is implemented.

@EZoni EZoni requested a review from dpgrote October 21, 2024 21:54
Copy link
Member

@dpgrote dpgrote left a comment

Choose a reason for hiding this comment

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

This looks good, thanks!
Changing the names of the routines is fine since this is only a temporary fix until they are ultimately removed.

@EZoni EZoni merged commit 05e09b1 into ECP-WarpX:development Oct 21, 2024
37 checks passed
@EZoni EZoni deleted the weekly-update branch October 21, 2024 23:22
dpgrote pushed a commit to dpgrote/WarpX that referenced this pull request Oct 23, 2024
- Weekly update to latest AMReX:
```console
./Tools/Release/updateAMReX.py
```
- Weekly update to latest pyAMReX:
```console
./Tools/Release/updatepyAMReX.py
```
- Weekly update to latest PICSAR (no changes):
```console
./Tools/Release/updatePICSAR.py
```
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component: third party Changes in WarpX that reflect a change in a third-party library
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants