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

Bug/Fix feedstock mac #790

Merged
merged 2 commits into from
Oct 16, 2024
Merged

Bug/Fix feedstock mac #790

merged 2 commits into from
Oct 16, 2024

Conversation

figueroa1395
Copy link
Contributor

@figueroa1395 figueroa1395 commented Oct 16, 2024

Feedstock is crashing after 1.9.72 on Mac OS. From the error log in the Azure pipelines, this can be traced back to #778, specifically to the test_simple_permanent_update test.

In the test, a single permanent update was done via model.update(update_data=update_batch), where update_batch was a many-scenario batch dataset, rather than a single scenario one, which is expected to work. However, due to the crash in the feedstock's CI, it was found that this functionality is not properly tested in the core.

In this PR, the python test test_simple_permanent_update is changed to handle a single scenario permanent update to confirm this was the root of the issue and tests are added in the core to test the intended functionality.

I expect two possible outcomes:

  • CI passes here: We merge and check if it now passes in feedstock. If it passes in the feedstock, the bug is either in the python interface with the C API (unlikely) or a memory alignment problem (most likely), as it was found in b3ca98f and reproduced in Repro case for buffer alignment  #769. The core should be good.
  • CI fails here: Since the new tests pass in my local, the core implementation should be fine and I would expect this to fail on the Mac CI, and would have to assess in that case.

Signed-off-by: Santiago Figueroa Manrique <santiago.figueroa.manrique@alliander.com>
Signed-off-by: Santiago Figueroa Manrique <santiago.figueroa.manrique@alliander.com>
Copy link

@figueroa1395 figueroa1395 marked this pull request as ready for review October 16, 2024 09:36
nitbharambe
nitbharambe previously approved these changes Oct 16, 2024
@nitbharambe nitbharambe dismissed their stale review October 16, 2024 11:44

Minor comment

@nitbharambe nitbharambe enabled auto-merge October 16, 2024 13:24
@nitbharambe nitbharambe added this pull request to the merge queue Oct 16, 2024
Merged via the queue into main with commit 813eff1 Oct 16, 2024
26 checks passed
@nitbharambe nitbharambe deleted the bug/fix-feedstock-mac branch October 16, 2024 14:07
@mgovers mgovers mentioned this pull request Nov 5, 2024
27 tasks
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