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

fix: make sure that suction pressure is less than or equal to dischar… #104

Merged
merged 1 commit into from
Jun 30, 2023

Conversation

olelod
Copy link
Contributor

@olelod olelod commented Jun 29, 2023

…ge pressure for compressor train

Why is this pull request needed?

Suction pressure higher than discharge pressure gave negative enthalpy change over a compressor train and hence negative power.

What does this pull request change?

Validates that suction pressure is less than or equal to the discharge pressure for a compressor train. If the suction pressure is higher than the discharge pressure a failure status INVALID_SUCTION_PRESSURE_INPUT is given. In reality it could be either the suction pressure being too high or the discharge pressure being too low, but that will be up to the user to figure out...it can be hard to automatically detect which one it is.

@@ -209,7 +209,19 @@ def _validate_operational_conditions(
discharge_pressure = np.where(
np.logical_and(np.min(rate, axis=0) <= 0, discharge_pressure <= 0), 1, discharge_pressure
)

if not np.all(discharge_pressure >= suction_pressure):
Copy link
Collaborator

Choose a reason for hiding this comment

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

will this yield failure_status 0 and suction pressure or invalid discharge pressure (not sure which one is wrong in such a case...)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not straight forward to know if it is the suction or discharge pressure which is wrong. The validation will return INVALID_SUCTION_PRESSURE_INPUT.

np.logical_and(np.min(rate, axis=0) <= 0, suction_pressure > discharge_pressure),
discharge_pressure,
suction_pressure,
)
Copy link
Collaborator

Choose a reason for hiding this comment

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

fairly complicated to read this numpy juggling. can it be extracted in a separate function and given some tests?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Since we are now only validating input when rate != 0 the numpy juggling can be simplified a bit, it seems.

Copy link
Collaborator

Choose a reason for hiding this comment

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

nice catch :)

@olelod olelod force-pushed the make-sure-suction-pressure-le-discharge-pressure branch from 156cf19 to 18a4ad5 Compare June 29, 2023 13:30
@olelod olelod marked this pull request as ready for review June 29, 2023 13:36
@olelod olelod requested a review from a team as a code owner June 29, 2023 13:36
f" and rate is set to zero."
)
rate = np.where(discharge_pressure < suction_pressure, 0, rate)
suction_pressure = np.where(suction_pressure > discharge_pressure, discharge_pressure, suction_pressure)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why do we change suction pressure to discharge pressure here when it is not calculated (ie suction > discharge)? Will it not be confusing?

Copy link
Collaborator

@TeeeJay TeeeJay left a comment

Choose a reason for hiding this comment

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

Good now, just want to know/clarify why we change the pressure when returning to user (I guess?). Should possibly notify user about this change as well?

@olelod
Copy link
Contributor Author

olelod commented Jun 29, 2023

Good now, just want to know/clarify why we change the pressure when returning to user (I guess?). Should possibly notify user about this change as well?

Good catch. On second thought I guess we don't need to change them. I was coloured by the similar checks against negative pressure - there we have to change them due to initialisation of fluids in NeqSim.

…ge pressure for compressor train

chore: add test of validation of operational conditions when suction pressure exceeds discharge pressure
@olelod olelod force-pushed the make-sure-suction-pressure-le-discharge-pressure branch from 18a4ad5 to 8e683e6 Compare June 30, 2023 07:36
@olelod olelod merged commit d218273 into main Jun 30, 2023
4 checks passed
@olelod olelod deleted the make-sure-suction-pressure-le-discharge-pressure branch June 30, 2023 07:56
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.

2 participants