Skip to content

[OneD] Fix issue in Sim1D::setFixedTemperature#936

Merged
speth merged 2 commits into
Cantera:mainfrom
ischoegl:fix-osx-failure-in-hdf
Jan 19, 2021
Merged

[OneD] Fix issue in Sim1D::setFixedTemperature#936
speth merged 2 commits into
Cantera:mainfrom
ischoegl:fix-osx-failure-in-hdf

Conversation

@ischoegl
Copy link
Copy Markdown
Member

@ischoegl ischoegl commented Sep 13, 2020

Changes proposed in this pull request

  • Avoid exact comparison of floats as criterion whether new point needs to be added.

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

Fixes #934

Checklist

  • There is a clear use-case for this code change
  • The commit message has a short title & references relevant issues
  • Build passes (scons build & scons test) and unit tests address code coverage
  • The pull request is ready for review

@santoshshanbhogue
Copy link
Copy Markdown
Contributor

Yup, can confirm that this fixes #934 on OSX 10.15.16. Thanks @ischoegl

`


*** Testing Summary ***


Tests passed: 1125
Up-to-date tests skipped: 0
Tests failed: 1
Failed tests:
- python:test_onedim.TestIonBurnerFlame.test_ion_profile


`

@ischoegl
Copy link
Copy Markdown
Member Author

ischoegl commented Sep 13, 2020

@santoshshanbhogue ... thank you for confirming! Per discussion, this was not caught by CI. The one remaining failure pertains to #935, which I will not address in this PR.

Copy link
Copy Markdown
Member

@speth speth left a comment

Choose a reason for hiding this comment

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

Adding a tolerance here makes sense to me, but I had a couple of questions about the implementation.

Comment thread src/oneD/Sim1D.cpp Outdated
Comment thread src/oneD/Sim1D.cpp Outdated
@ischoegl
Copy link
Copy Markdown
Member Author

ischoegl commented Dec 6, 2020

@speth ... I decided to go over the code to simplify / make it more readable.

As with the other PR, I can rebase once #946 is merged.

@codecov
Copy link
Copy Markdown

codecov Bot commented Dec 6, 2020

Codecov Report

Merging #936 (7e949df) into main (d8e62ad) will decrease coverage by 0.00%.
The diff coverage is 90.90%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #936      +/-   ##
==========================================
- Coverage   71.22%   71.21%   -0.01%     
==========================================
  Files         377      377              
  Lines       46272    46274       +2     
==========================================
  Hits        32955    32955              
- Misses      13317    13319       +2     
Impacted Files Coverage Δ
src/oneD/Sim1D.cpp 81.10% <90.90%> (-0.48%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update d8e62ad...f6c4c5b. Read the comment docs.

Avoid exact comparison of floats as criterion whether new point needs
to be added.
@ischoegl ischoegl force-pushed the fix-osx-failure-in-hdf branch from 7126a1c to 7e949df Compare December 11, 2020 03:03
@ischoegl ischoegl mentioned this pull request Dec 11, 2020
4 tasks
@ischoegl ischoegl requested a review from speth December 11, 2020 12:31
@ischoegl ischoegl force-pushed the fix-osx-failure-in-hdf branch from 7e949df to f6c4c5b Compare January 18, 2021 23:04
Copy link
Copy Markdown
Member

@speth speth 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. I think this can be merged as soon as the tests pass.

@speth speth merged commit e3d06fa into Cantera:main Jan 19, 2021
@ischoegl ischoegl deleted the fix-osx-failure-in-hdf branch January 29, 2021 18:48
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.

test_onedim.TestFreeFlame fails on OSX 10.15.6

4 participants