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

fixed issue 992 #1065

Merged
merged 1 commit into from
Jun 23, 2021
Merged

fixed issue 992 #1065

merged 1 commit into from
Jun 23, 2021

Conversation

jongyoonbae
Copy link
Contributor

Changes proposed in this pull request

  • Fixed matlab documentation error for entropy_mole in the interface: unit changed from J/kg to J/mol-K.

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

Closes #992

If applicable, provide an example illustrating new features this pull request is introducing

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

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 for the proposed fix! The hyphen here is consistent with J/kg-K in entropy_mass.m. However, could you double-check whether this is kmol or mol in MATLAB? (at least in Python, the corresponding method outputs the former, with units given as J/kmol/K)

@decaluwe
Copy link
Member

@ischoegl is correct - this is my fault, as @jongyoonbae was simply following my comment in the issue.

@jongyoonbae - if you edit this to be J/kmol-K you just have to add, commit, and then push back to your origin remote. The new change will automatically be folded into your pull request.

@bryanwweber
Copy link
Member

@jongyoonbae rather than making a new commit, please use the --amend that I showed earlier to amend the existing commit and force push 😊 Thanks!

@jongyoonbae
Copy link
Contributor Author

I just did. Thank you for reviewing and commenting!

@ischoegl
Copy link
Member

@jongyoonbae ... Thank you for the fix!

@ischoegl ischoegl merged commit fceb7cd into Cantera:main Jun 23, 2021
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.

Documentation error in Matlab entropy_mole
4 participants