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

Add a linspace-like function in AMReX_Algorithm.H #3698

Merged

Conversation

lucafedeli88
Copy link
Contributor

Summary

I would like to propose to add a linspace-like function in AMReX_Algorithm.H , since filling a container with equally spaced numbers is a task that occurs rather frequently. This PR proposes a possible implementation.
The container is modified in place only if it has at least 2 elements.

Additional background

Checklist

The proposed changes:

  • fix a bug or incorrect behavior in AMReX
  • 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

@lucafedeli88
Copy link
Contributor Author

Hello everyone. Do you think that it could be interesting to offer this function in AMReX ?
It would be quite useful in a radiation module that we are currently developing for the WarpX code.

@WeiqunZhang
Copy link
Member

I think such a function is useful. I wonder if we should make sure that both typename std::iterator_traits<ItType>::value_type and ValType are floating point types. Otherwise calls like linspace(..., 0, 3) will not work as expected.

const std::ptrdiff_t count = last-first;
if (count >= 2){
const auto delta = (stop - start)/(count - 1);
for (int i = 0; i < count-1; ++i){
Copy link
Member

Choose a reason for hiding this comment

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

Could use std::ptrdiff_t instead of int.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's a good idea. I've replaced int with std::ptrdiff_t

@lucafedeli88
Copy link
Contributor Author

I think such a function is useful. I wonder if we should make sure that both typename std::iterator_traits<ItType>::value_type and ValType are floating point types. Otherwise calls like linspace(..., 0, 3) will not work as expected.

I agree!
Is it OK like that?

    template<typename ItType, typename ValType,
        typename std::enable_if<std::is_floating_point<typename ItType::value_type>::value,int>::type = 0,
        typename std::enable_if<std::is_floating_point<ValType>::value,int>::type = 0>

It seems to work, although the syntax is quite verbose!

@@ -208,6 +208,22 @@ namespace amrex
#endif
}

template<typename ItType, typename ValType,
typename std::enable_if<std::is_floating_point<typename ItType::value_type>::value,int>::type = 0,
Copy link
Member

Choose a reason for hiding this comment

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

I think we need to use iterator_traits otherwise raw pointers would not work as ItType.

We could also combine the two enable_ifs into one.

Copy link
Member

Choose a reason for hiding this comment

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for the link. This version uses iterator_traits and combines the two enable_if

@WeiqunZhang WeiqunZhang merged commit 03f5936 into AMReX-Codes:development Jan 16, 2024
69 checks passed
WeiqunZhang pushed a commit that referenced this pull request Feb 14, 2024
## Summary

Similarly to what was done in
#3698, I would like to propose
to add a `logspace-like` function in `AMReX_Algorithm.H`. Indeed,
filling a container with logarithmically spaced numbers occurs rather
frequently. This PR proposes a possible implementation. As for
`linspace`, the container is modified (in place) only if it has at least
2 elements.

## Additional background

## 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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants