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

Fix some problems with ExtensibleRate #1494

Merged
merged 2 commits into from
May 29, 2023
Merged

Fix some problems with ExtensibleRate #1494

merged 2 commits into from
May 29, 2023

Conversation

speth
Copy link
Member

@speth speth commented May 27, 2023

Changes proposed in this pull request

  • Fix de-registration of "change callbacks" from Python. I'm pretty sure this is the bug causing the failures noted in Occasional segfault in ExtensibleRate tests #1489.
  • Fix working with locally-instantiated ExtensibleRate objects so they can also be retrieved from the Solution object

If applicable, fill in the issue number this pull request is fixing

Closes #1489

Checklist

  • The pull request includes a clear description of this code change
  • Commit messages have short titles and reference relevant issues
  • Build passes (scons build & scons test) and unit tests address code coverage
  • Style & formatting of contributed code follows contributing guidelines
  • The pull request is ready for review

speth added 2 commits May 27, 2023 11:56
Resolves a case where _SolutionBase objects that did not own the
underlying C++ Solution object were not de-registring their change
callbacks when the _SolutionBase object was being deleted.

Fixes Cantera#1489
@speth speth added the Python label May 27, 2023
@codecov
Copy link

codecov bot commented May 27, 2023

Codecov Report

Merging #1494 (eef6f0c) into main (0d449ca) will increase coverage by 0.00%.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##             main    #1494   +/-   ##
=======================================
  Coverage   69.50%   69.50%           
=======================================
  Files         377      377           
  Lines       57993    57995    +2     
  Branches    20693    20693           
=======================================
+ Hits        40310    40312    +2     
  Misses      14732    14732           
  Partials     2951     2951           
Impacted Files Coverage Δ
interfaces/cython/cantera/reaction.pyx 82.22% <100.00%> (+0.03%) ⬆️
interfaces/cython/cantera/solutionbase.pyx 89.37% <100.00%> (+0.03%) ⬆️

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

Copy link
Member

@ischoegl ischoegl left a comment

Choose a reason for hiding this comment

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

Thanks, @speth! Looks good to me.

@ischoegl ischoegl merged commit b7d4c83 into Cantera:main May 29, 2023
@speth speth deleted the fix-1489 branch July 23, 2024 15:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Occasional segfault in ExtensibleRate tests
2 participants