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

Remove m_singleParameterResizeIndex from ArrayView to reduce memory. #311

Merged
merged 10 commits into from
Feb 6, 2024

Conversation

dkachuma
Copy link
Contributor

@dkachuma dkachuma commented Jan 10, 2024

  • m_singleParameterResizeIndex is replaced by a constant value hard-coded to 0.

  • Two interface methods are removed:

    • int ArrayView::getSingleParameterResizeIndex() const
    • void Array::setSingleParameterResizeIndex( int const index )

    Not sure if these are used elsewhere but I managed to build geos without these.

There is a loss of some functionality here. In examples/exampleArray.cpp for instance, you could do

LvArray::Array< int > array( 5, 6 );
array.setSingleParameterResizeIndex( 1 );
array.resize( 3 );

which would reshape the array to 5x3. This cannot be done anymore.

- `m_singleParameterResizeIndex` is replaced by a `constexpr` value hard-coded to 0.
- Two interface methods are removed:
  *  `int ArrayView::getSingleParameterResizeIndex() const`
  * `void Array::setSingleParameterResizeIndex( int const index )`
@dkachuma dkachuma self-assigned this Jan 10, 2024
@dkachuma dkachuma marked this pull request as ready for review January 16, 2024 21:34
src/Array.hpp Outdated Show resolved Hide resolved
src/ArrayView.hpp Outdated Show resolved Hide resolved
src/python/PyArray.cpp Outdated Show resolved Hide resolved
Copy link
Collaborator

@corbett5 corbett5 left a comment

Choose a reason for hiding this comment

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

Looks good to me! If the tests are passing (the python interface tests as well) it is a welcome change!

src/MallocBuffer.hpp Show resolved Hide resolved
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants