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

Add flooding #1

Open
wants to merge 74 commits into
base: master
Choose a base branch
from
Open

Add flooding #1

wants to merge 74 commits into from

Conversation

ibell
Copy link
Owner

@ibell ibell commented Apr 25, 2015

No description provided.

dziviani added 2 commits April 21, 2015 10:42
Create a new StateFlooded class derived from CoolProp State class
@ibell
Copy link
Owner Author

ibell commented Apr 25, 2015

In FloodState, you should almost never be calling PropsSI, you should always try to call self.pAS.keyed_output instead


def T_crit(self,Ref): # Critical Temperature of the refrigerant [K] - From coolprop -> Props(Fluid,PropName)

if Ref=='R245fa':
Copy link
Owner Author

Choose a reason for hiding this comment

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

Should not do it like this, you should call

self.pAS.keyed_output(iTcrit)

@ibell
Copy link
Owner Author

ibell commented May 7, 2015

You need to implement all the methods of the base class - get_T, get_p, etc. You'll notice that in the pdsim code, we call the get_T, get_rho.. methods in order to not need to pass at all into the python layer. If you don't implement these functions, it will call the base class function, which is not what you want at all.


#TODO: check consistency with StateFlooded.pyx get_h_m or get_h. Maybe we should differentiate from standard State class and use _m to indicate mixture properties

cdef class State_Flooded:
Copy link
Owner Author

Choose a reason for hiding this comment

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

MUST(!) derive from State class. That is the whole point.

Copy link
Owner Author

Choose a reason for hiding this comment

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

Also, better to name StateFlooded - that is more standard. Class definitions should be CamelCase, without underscores.

@ibell
Copy link
Owner Author

ibell commented May 7, 2015

You should try to get your code to compile now. You will need to add your file in setup.py (in the list of pyx files).

if Ref=='R245fa':
HEOS = CoolProp.AbstractState('HEOS',Ref)
self.p = HEOS.keyed_output(CoolProp.pcrit)
elif Ref=='R410A':
Copy link
Owner Author

Choose a reason for hiding this comment

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

Don't do it like this - in the init function for the class you need to create the pointer to the AbstractState as is done in the State class in CoolProp. Otherwise this makes a local definition for HEOS which is definitely not what you want.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Not sure if it is necessary to define p_crit and T_crit within StateFlooded class, since it can be called from PDSim directly from AbstractState class.

@ibell ibell mentioned this pull request Jun 19, 2016
ibell pushed a commit that referenced this pull request Apr 23, 2020
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