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

Vapour Quality and State Reporting #592

Merged
merged 9 commits into from
Jan 22, 2025
Merged

Vapour Quality and State Reporting #592

merged 9 commits into from
Jan 22, 2025

Conversation

tlmerbecks
Copy link
Contributor

Ciao @fwitte,

I noticed that the property reporting for Connections does not include the vapour quality or the state, hence the overall aim of this pull request is to include them in the table of properties for Connections.

I implemented the following changes:

  • Aligned the calculation of the vapour quality for the CoolPropWrapper with that of the IAPWSWrapper. Currently the CoolPropWrapper reports a vapour quality of -1 for all single phase states (irrespective of it being gas or liquid). With the changes below, the vapour quality is now reported as 0 for liquids and 1 for gases, for subcritical conditions.
  • Added a state_ph function to the base FluidPropertiesWrapper, the CoolPropWrapper and the IAPWSWrapper. Unfortunately, I lack expertise with Pyromat and so have not done this... yet)
  • Extended the fluid_properties_data (global_vars) with a state properties field
  • Added an exception rule for the unit conversion of the new state property
  • Added the vapour quality and state to the reported Connection results

I hope the above is clear enough. Is there anything else that would be helpful?

@pep8speaks
Copy link

pep8speaks commented Jan 5, 2025

Hello @tlmerbecks! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:

Line 2577:80: E501 line too long (89 > 79 characters)

Line 158:1: E302 expected 2 blank lines, found 1

Line 246:80: E501 line too long (90 > 79 characters)
Line 382:22: E271 multiple spaces after keyword

Comment last updated at 2025-01-22 20:13:24 UTC

@tlmerbecks
Copy link
Contributor Author

Looks like my PR was not overly successful... Unfortunately, I am somewhat struggling to understand exactly what tests have failed

@fwitte
Copy link
Member

fwitte commented Jan 6, 2025

@tlmerbecks, thank you for your contribution. I will have a look today evening, my first guess is, that it is because the state keyword is already utilized for different context (may be 'l' or 'g' for liquid/gas) in order to help convergence. If specified, the solver forces the state to be liquid or gaseous. Adding 'tp' is a nice touch, but for the rest I have to look in detail into your changes. I'll leave you some feedback later :).

@tlmerbecks
Copy link
Contributor Author

tlmerbecks commented Jan 6, 2025

Ciao @fwitte,

that may well be it...

That being said, I was a bit confused that the state variable is not defined as part of the get_parameters function, hence I added it as a field there - am I missing something?

Perhaps this was unclear in the original PR, most of the additions are simply to allow to the state to be calculated mid- or post-solve. This then allows the state to be utilised as part of Component calculations (I am currently working on a separate PR for a new Turbine component that handles wet-expansion, as you would see in a geothermal or nuclear power plant), or so that it can be reported alongside the pressure and enthalpy in the Connection results table.

@tlmerbecks
Copy link
Contributor Author

Actually, it seems as though the state variable is not declared via the dictionary generated by the get_parameters function, instead it is declared directly. However, with my additions, it would be simply overwritten when all other parameters are set.

class Connection:
        def __init__(self, ...):

            # some other code

            # set default values for kwargs
            self.property_data = self.get_parameters()  # I defined the state field in here
            self.parameters = {
                k: v for k, v in self.get_parameters().items()
                if hasattr(v, "func") and v.func is not None
            }
            self.state = dc_simple()  # currently tespy declares the state variable here
            self.property_data0 = [x + '0' for x in self.property_data.keys()]
            self.__dict__.update(self.property_data)  # <-- overwrites the directly declared variable with my definition...

            # some other code

@tlmerbecks tlmerbecks marked this pull request as draft January 7, 2025 19:02
@tlmerbecks tlmerbecks marked this pull request as ready for review January 12, 2025 14:30
@fwitte fwitte merged commit df4be96 into oemof:dev Jan 22, 2025
3 of 7 checks passed
@fwitte
Copy link
Member

fwitte commented Jan 22, 2025

@tlmerbecks, thank you for your contribution! I updated the changelog, bumped the version and removed that commeted section in the wrappers.py Q_ph method of the CoolPropWrapper.

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.

3 participants