Skip to content

Fix an issue with empty preallocation #113

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Merged
merged 3 commits into from
Jun 16, 2024

Conversation

alcrene
Copy link
Contributor

@alcrene alcrene commented Jun 9, 2024

The original logic of grow_append was to extend data by 10% of its length.
This is a problem with the original data length is 0, since it then never extends.

This PR amends grow_append to always extend by at least 10 elements.
It also updates the _growing test to test both 0 and non-zero preallocation.

Alexandre René and others added 3 commits June 9, 2024 14:27
Original logic in `grow_append` was to extend data by 10% of its length.
This is a problem with the original data length is 0, since it then
never extends.
This commit amends `grow_append` to always extend by at least 10 elements.
`grow_append` cannot know if ``preallocate = 0`` was used: it only
looks at the `rigid` value to determine how to append.
Because of this, will always fail when we use `preallocate = 0` with
tensor variables, since then the shapes of `target` and `extension`
don’t match.
A simple fix is to simply deactivate the special behavior for
`preallocate = 0`.

- This commit extends `test_growing` with a `preallocate` parameter,
  so that we test both cases where it is 0 and positive.
- We also fix `test_growing` to match the new behavior of `grow_append`
  introduced in #ea812b0, where data arrays always grow by at least 10.
Copy link
Member

@michaelosthege michaelosthege left a comment

Choose a reason for hiding this comment

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

thanks for the fix!

@michaelosthege michaelosthege merged commit b9704f3 into pymc-devs:main Jun 16, 2024
1 check passed
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants