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

Update C++ and Fortran example docstrings #983

Merged
merged 5 commits into from
Apr 6, 2021

Conversation

speth
Copy link
Member

@speth speth commented Feb 24, 2021

This PR is a companion to Cantera/cantera-website#136.

Changes proposed in this pull request

  • Standardize the header comments of C++/Fortran examples to make them play nice with the website
  • Add missing license info to C++/Fortran examples
  • A little bit of miscellaneous cleanup in these examples

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

Fixes #

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

@codecov
Copy link

codecov bot commented Feb 24, 2021

Codecov Report

Merging #983 (c459be9) into main (e70371a) will decrease coverage by 0.00%.
The diff coverage is 90.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #983      +/-   ##
==========================================
- Coverage   71.60%   71.60%   -0.01%     
==========================================
  Files         360      360              
  Lines       45719    45718       -1     
==========================================
- Hits        32736    32735       -1     
  Misses      12983    12983              
Impacted Files Coverage Δ
samples/cxx/combustor/combustor.cpp 93.24% <ø> (ø)
samples/cxx/custom/custom.cpp 100.00% <ø> (ø)
samples/cxx/demo/demo.cpp 95.12% <ø> (ø)
samples/cxx/flamespeed/flamespeed.cpp 94.59% <ø> (ø)
samples/cxx/gas_transport/gas_transport.cpp 89.74% <ø> (ø)
samples/cxx/kinetics1/example_utils.h 100.00% <ø> (ø)
samples/cxx/kinetics1/kinetics1.cpp 88.63% <ø> (ø)
samples/cxx/openmp_ignition/openmp_ignition.cpp 88.37% <ø> (ø)
samples/cxx/rankine/rankine.cpp 93.47% <ø> (ø)
samples/f77/demo_ftnlib.cpp 31.85% <ø> (ø)
... and 3 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 e70371a...4cdf97a. Read the comment docs.

Copy link
Contributor

@12Chao 12Chao left a comment

Choose a reason for hiding this comment

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

The commits look good to me, and I didn't find anything wrong.

Comment on lines +7 to +8
* equations are custom-implemented, using Cantera's interface to CVODES to
* integrate the equations.
Copy link
Contributor

@jongyoonbae jongyoonbae Mar 30, 2021

Choose a reason for hiding this comment

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

"Cantera's interface to CVODES to integrate" -> Do you mean "Cantera's interface CVODES to integrate"?

Copy link
Member

Choose a reason for hiding this comment

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

Thanks @jongyoonbae! I think this is correct as written. Cantera provides an interfaces that connects to CVODES, which is actually does the numerical integration.

Copy link
Contributor

@jongyoonbae jongyoonbae left a comment

Choose a reason for hiding this comment

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

Found one potential typo, but looks good otherwise.

Copy link
Member

@bryanwweber bryanwweber 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, thanks @speth!

@bryanwweber bryanwweber merged commit b42989f into Cantera:main Apr 6, 2021
@speth speth deleted the example-docstrings branch April 6, 2021 14:19
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.

4 participants