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

Miscellanous small improvements #1140

Merged
merged 7 commits into from
Nov 20, 2021
Merged

Miscellanous small improvements #1140

merged 7 commits into from
Nov 20, 2021

Conversation

speth
Copy link
Member

@speth speth commented Nov 20, 2021

Changes proposed in this pull request

  • Avoid repeated regex compilation that was slowing down mechanism import
  • Switch high-debug-level flame solver output from XML to YAML
  • Remove unused variable OneDim::m_solve_time
  • Use ruamel.yaml instead of the out-of-date ruamel_yaml
  • Add Mu0Poly and Nasa9PolyMultiTempRegion to Python docs
  • Improve error messages from fpValueCheck
  • Fix a bunch of miscellaneous spelling errors

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

Compiling this regex every time a Units object was created from a
string was resulting in a noticeable slowdown when importing larger
mechanisms.
The ruamel_yaml package name was only ever used by Anaconda. They have
now adopted the ruamel.yaml name. The ruamel_yaml package is no longer
up-to-date, so we should default to using ruamel.yaml and only fall
back to ruamel_yaml if necessary, rather than the other way around.
Printing the input string that causes the error helps users find
problematic input.
@codecov
Copy link

codecov bot commented Nov 20, 2021

Codecov Report

Merging #1140 (ebce161) into main (032de5e) will not change coverage.
The diff coverage is 35.29%.

Impacted file tree graph

@@           Coverage Diff           @@
##             main    #1140   +/-   ##
=======================================
  Coverage   73.49%   73.49%           
=======================================
  Files         365      365           
  Lines       48187    48187           
=======================================
  Hits        35417    35417           
  Misses      12770    12770           
Impacted Files Coverage Δ
include/cantera/base/Units.h 100.00% <ø> (ø)
include/cantera/equil/MultiPhase.h 100.00% <ø> (ø)
include/cantera/kinetics/Kinetics.h 56.75% <ø> (ø)
include/cantera/numerics/FuncEval.h 83.33% <ø> (ø)
include/cantera/oneD/OneDim.h 52.45% <ø> (ø)
include/cantera/thermo/ThermoPhase.h 32.46% <ø> (ø)
include/cantera/zeroD/Reactor.h 63.15% <ø> (ø)
include/cantera/zeroD/flowControllers.h 61.76% <ø> (ø)
src/base/application.h 76.92% <ø> (ø)
src/kinetics/BulkKinetics.cpp 91.00% <ø> (ø)
... and 10 more

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 032de5e...ebce161. Read the comment docs.

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.

Thank you, @speth - this covered a lot of ground! I only have a minor comment.

src/base/stringUtils.cpp Show resolved Hide resolved
@ischoegl ischoegl merged commit 2d211db into Cantera:main Nov 20, 2021
@speth speth deleted the misc-cleanup branch July 23, 2024 15:37
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