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

Particle Container: Runtime Arguments #222

Merged
merged 4 commits into from
Apr 2, 2024

Conversation

ax3l
Copy link
Member

@ax3l ax3l commented Nov 6, 2023

Follow-up to #220

  • Particle Runtime Args: New Bindings
  • ParticleContainer Tests: w/ Runtime Attributes

X-ref: AMReX-Codes/amrex#3615

@ax3l ax3l requested a review from atmyers November 6, 2023 22:21
@ax3l ax3l added the enhancement New feature or request label Nov 6, 2023
tests/test_particleContainer.py Outdated Show resolved Hide resolved
Comment on lines +64 to +66
.def("assign", [](PODVector_type & pv, T const & value){
pv.assign(pv.size(), value);
}, py::arg("value"), "assign the same value to every element")
Copy link
Member Author

@ax3l ax3l Nov 6, 2023

Choose a reason for hiding this comment

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

Overloaded because vector.assign(vector.size(), 42) to set a value is a bit cumbersome.
https://github.com/AMReX-Codes/amrex/blob/47347f785f5c26f1f9e65bef8e10f2d383406ef7/Src/Base/AMReX_PODVector.H#L480-L513

Copy link
Member Author

Choose a reason for hiding this comment

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

Could also call this set_value in symmetry with MultiFab, please let me know what you prefer @WeiqunZhang @atmyers

Copy link
Member Author

Choose a reason for hiding this comment

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

@@ -192,6 +192,24 @@ void make_ParticleContainer_and_Iterators (py::module &m, std::string allocstr)
.def_property_readonly_static("NArrayReal", [](const py::object&){return ParticleContainerType::NArrayReal; })
.def_property_readonly_static("NArrayInt", [](const py::object&){return ParticleContainerType::NArrayInt; })

.def_property_readonly("num_runtime_real_comps", &ParticleContainerType::NumRuntimeRealComps)
.def_property_readonly("num_runtime_int_comps", &ParticleContainerType::NumRuntimeIntComps)
.def_property_readonly("num_real_comps", &ParticleContainerType::NumRealComps)

Choose a reason for hiding this comment

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

So once this is merged we can clean up the WarpXparticleContainer.cpp file, for example the line at https://github.com/ECP-WarpX/WarpX/blob/b3ba07df16de4e9736b01a8cc7024740714dcad6/Source/Python/Particles/WarpXParticleContainer.cpp#L88, right?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, def!

@ax3l ax3l requested a review from dpgrote November 7, 2023 02:15
@ax3l
Copy link
Member Author

ax3l commented Nov 7, 2023

Looks like something does not deallocate PC runtime components in time.

@ax3l ax3l force-pushed the development branch 7 times, most recently from 6c95aa1 to b35493b Compare November 30, 2023 20:48
@ax3l ax3l force-pushed the topic-test-runtime-args branch 2 times, most recently from dc334e7 to 630a156 Compare November 30, 2023 21:47
@ax3l ax3l force-pushed the topic-test-runtime-args branch 2 times, most recently from 35c19c9 to e44ff54 Compare November 30, 2023 22:12
tests/test_particleContainer.py Outdated Show resolved Hide resolved
@ax3l ax3l force-pushed the topic-test-runtime-args branch 2 times, most recently from faf529e to b534827 Compare February 16, 2024 21:53
tests/test_particleContainer.py Outdated Show resolved Hide resolved
atmyers pushed a commit to AMReX-Codes/amrex that referenced this pull request Mar 29, 2024
## Summary

When adding new `Real`/`Int` runtime components, they could be made
available without additional calls.

The cost should be the same as calling it explicitly later, but
clarifies the usage. Alternatively, we should add API contract details
to the `AddRealComp`/`AddIntComp` doc strings to make sure people use it
right.

## Additional background

- AMReX-Codes/pyamrex#220
- AMReX-Codes/pyamrex#222

## Checklist

The proposed changes:
- [ ] fix a bug or incorrect behavior in AMReX
- [x] add new capabilities to AMReX
- [ ] changes answers in the test suite to more than roundoff level
- [ ] are likely to significantly affect the results of downstream AMReX
users
- [ ] include documentation in the code and/or rst files, if appropriate
@ax3l ax3l mentioned this pull request Mar 29, 2024
2 tasks
@ax3l ax3l force-pushed the topic-test-runtime-args branch 3 times, most recently from 7f3d297 to 1e44d7b Compare April 1, 2024 17:09
WeiqunZhang pushed a commit to AMReX-Codes/amrex that referenced this pull request Apr 2, 2024
## Summary

```
vector.assign(vector.size(), 42);
```
is a bit verbose for a standard operation, even if it mirrors
https://en.cppreference.com/w/cpp/container/vector/assign

Add another overload similar to `setVal(ue)` used in other AMReX
containers.

## Additional background

AMReX-Codes/pyamrex#222 (comment)

## Checklist

The proposed changes:
- [ ] fix a bug or incorrect behavior in AMReX
- [x] add new capabilities to AMReX
- [ ] changes answers in the test suite to more than roundoff level
- [ ] are likely to significantly affect the results of downstream AMReX
users
- [ ] include documentation in the code and/or rst files, if appropriate
@ax3l ax3l merged commit 304698a into AMReX-Codes:development Apr 2, 2024
19 checks passed
@ax3l ax3l deleted the topic-test-runtime-args branch April 2, 2024 20:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants