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

IGMIX does not include entropy of mixing #92

Closed
chmarti1 opened this issue Jul 29, 2024 · 3 comments
Closed

IGMIX does not include entropy of mixing #92

chmarti1 opened this issue Jul 29, 2024 · 3 comments
Assignees

Comments

@chmarti1
Copy link
Owner

Users should be aware that igmix does not correctly add the "entropy of mixing" to its calculations. This winds up being a constant added to all entropy calculations. It will not affect the results of any engine or cycle calculations for the reasons already described in the FAQ. However, users doing mixture work might notice some small discrepancies (e.g. about 2% for atmospheric temperature air).

As of version v2.2.4, if users are doing work with variable gas mixtures, it probably doesn't make sense to use the igmix class anyway, since it only allows static mixtures. On the other hand, if mixtures are constant, then the offset will not be important either. That's probably why this has never been a problem.

The entropy of mixing term will be added correctly in the next release, which may cause a small change in igmix class entropy values, but should not affect any cycle calculations.

@chmarti1 chmarti1 added the bug label Jul 29, 2024
@chmarti1 chmarti1 self-assigned this Jul 29, 2024
@jranalli
Copy link
Collaborator

jranalli commented Aug 2, 2024

This one has led to failures in our test suite for air entropy. Those tests were all based on PM-generated reference states just to monitor changes to code. I will update those reference values in the test suite. Do you have the entropy calculation "finalized" in the igtools branch, or should I hang tight for that one?

This raises a good question though of whether we have an external reference for air entropies that we could refer to rather than just ensuring the internal consistency?

jranalli added a commit that referenced this issue Aug 2, 2024
jranalli added a commit that referenced this issue Aug 2, 2024
…l iteration so that we yield correct values when adding it back on later. Relevant for fixing issues raised during the revisions to allow for #92.
@jranalli
Copy link
Collaborator

jranalli commented Aug 2, 2024

Alright, there are two separate branches I added from the igtools branch.

igtools-testsfix updates nothing but the test suite to accommodate expected behavior with changes introduced by igtools:

  • allow h&s, e&s to be property pairs for ideal gases
  • update the current reference states for igmix to include smix values.

There are a number of tests that fail, meaning there were bugs, so the branch igtools-bugfixes actually resolves those to get everything back to a passing state. @chmarti1 I know you like to work out these issues yourself, which is why I put it into a separate branch. I'll trim the branch if you end up implementing these fixes separately, but in the meantime hopefully the line diff's can at least help you ID the needed changes.

@chmarti1
Copy link
Owner Author

I've pushed my fixes to the master branch as v2.2.5 with the intention of letting it serve as a beta release. I figured it would break the test code since these features are fundamentally dissimilar to the historical behavior, so I used my typical manual command-line tests to confirm that the mixtures are working correctly.

I'll hold back an update of the python package index until we've had a chance to talk in more detail.

jranalli added a commit that referenced this issue Sep 30, 2024
jranalli added a commit that referenced this issue Sep 30, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants