Skip to content

fixed issue 992#1065

Merged
ischoegl merged 1 commit intoCantera:mainfrom
jongyoonbae:fix_992
Jun 23, 2021
Merged

fixed issue 992#1065
ischoegl merged 1 commit intoCantera:mainfrom
jongyoonbae:fix_992

Conversation

@jongyoonbae
Copy link
Copy Markdown
Contributor

@jongyoonbae jongyoonbae commented Jun 22, 2021

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

ischoegl
ischoegl requested changes Jun 22, 2021
View reviewed changes
Copy link
Copy Markdown
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
Copy Markdown
Member

decaluwe commented Jun 22, 2021

@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
Copy Markdown
Member

bryanwweber commented Jun 22, 2021

@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
Copy Markdown
Contributor Author

jongyoonbae commented Jun 23, 2021

I just did. Thank you for reviewing and commenting!

@ischoegl
Copy link
Copy Markdown
Member

ischoegl commented Jun 23, 2021

@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

Reviewers

@ischoegl ischoegl ischoegl approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Documentation error in Matlab entropy_mole

4 participants

@jongyoonbae @decaluwe @bryanwweber @ischoegl