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

MEP_Engine: Add HVAC Heat Equations #2967

Merged
merged 11 commits into from
Jan 5, 2023

Conversation

travispotterBH
Copy link

@travispotterBH travispotterBH commented Dec 22, 2022

Issues addressed by this PR

Closes #2737

Test files

Changelog

  • Compute.ProcessSensibleHeat - Added processes sensible heat calculation
  • Compute.ProcessLatentHeat - Added processes latent heat calculation
  • Compute.ProcessTotalHeat - Added processes total heat calculation
  • Compute.MassFlowRate - Added mass flow rate calculation calculation

Additional comments

@travispotterBH travispotterBH added the type:feature New capability or enhancement label Dec 22, 2022
@travispotterBH travispotterBH self-assigned this Dec 22, 2022
@travispotterBH travispotterBH changed the title MEP_Engine: Add HVAC Equations MEP_Engine: Add HVAC Heat Equations Dec 22, 2022
Copy link
Contributor

@FraserGreenroyd FraserGreenroyd left a comment

Choose a reason for hiding this comment

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

In addition to the comments, I'm not sure on the use of AirSide and WaterSide as sub-folders. Are we expecting many calculations for each type? Are there any other types of side we're not yet building up?

If we are needing AirSide and WaterSide, then we should have it match that in the namespace too which it doesn't currently.

@travispotterBH
Copy link
Author

In addition to the comments, I'm not sure on the use of AirSide and WaterSide as sub-folders. Are we expecting many calculations for each type? Are there any other types of side we're not yet building up?

If we are needing AirSide and WaterSide, then we should have it match that in the namespace too which it doesn't currently.

I could be swayed to different terminology. Wanting mostly to differentiate between equations that assume air as the process fluid vs those that assume water.

@travispotterBH
Copy link
Author

@alexissantella I generalized the equations. They work for both air and water, if they user supplies the proper specific heat and and fluid density values.

travispotterBH and others added 2 commits January 3, 2023 16:42
alexissantella
alexissantella previously approved these changes Jan 4, 2023
Copy link
Contributor

@alexissantella alexissantella left a comment

Choose a reason for hiding this comment

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

Reviewed equations for accuracy. It looks good to me.

@travispotterBH
Copy link
Author

@BHoMBot check required

@bhombot-ci
Copy link

bhombot-ci bot commented Jan 4, 2023

@travispotterBH to confirm, the following actions are now queued:

  • check code-compliance
  • check documentation-compliance
  • check project-compliance
  • check core
  • check null-handling
  • check serialisation
  • check versioning
  • check installer

@FraserGreenroyd
Copy link
Contributor

@BHoMBot check copyright-compliance
@BHoMBot check dataset-compliance

@bhombot-ci
Copy link

bhombot-ci bot commented Jan 4, 2023

@FraserGreenroyd to confirm, the following actions are now queued:

  • check copyright-compliance
  • check dataset-compliance

@FraserGreenroyd
Copy link
Contributor

@BHoMBot check unit-tests

@bhombot-ci
Copy link

bhombot-ci bot commented Jan 4, 2023

@FraserGreenroyd to confirm, the following actions are now queued:

  • check unit-tests

FraserGreenroyd
FraserGreenroyd previously approved these changes Jan 4, 2023
Copy link
Contributor

@FraserGreenroyd FraserGreenroyd left a comment

Choose a reason for hiding this comment

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

My comments have been address, @alexissantella is happy so this is all good to go.

@FraserGreenroyd
Copy link
Contributor

@BHoMBot check ready-to-merge

@bhombot-ci
Copy link

bhombot-ci bot commented Jan 4, 2023

@FraserGreenroyd to confirm, the following actions are now queued:

  • check ready-to-merge

@travispotterBH
Copy link
Author

@BHoMBot check unit-tests

@bhombot-ci
Copy link

bhombot-ci bot commented Jan 5, 2023

@travispotterBH to confirm, the following actions are now queued:

  • check unit-tests

There are 27 requests in the queue ahead of you.

@travispotterBH
Copy link
Author

@BHoMBot check required

@bhombot-ci
Copy link

bhombot-ci bot commented Jan 5, 2023

@travispotterBH to confirm, the following actions are now queued:

  • check code-compliance
  • check documentation-compliance
  • check project-compliance
  • check core
  • check null-handling
  • check serialisation
  • check versioning
  • check installer

@bhombot-ci
Copy link

bhombot-ci bot commented Jan 5, 2023

The check installer has already been run previously and recorded as a successful check. This check has not been run again at this time.

@travispotterBH
Copy link
Author

@BHoMBot check copyright-compliance
@BHoMBot check dataset-compliance
@BHoMBot check ready-to-merge

@bhombot-ci
Copy link

bhombot-ci bot commented Jan 5, 2023

@travispotterBH to confirm, the following actions are now queued:

  • check dataset-compliance
  • check ready-to-merge

There are 11 requests in the queue ahead of you.

@travispotterBH
Copy link
Author

@BHoMBot check copyright-compliance

@bhombot-ci
Copy link

bhombot-ci bot commented Jan 5, 2023

@travispotterBH to confirm, the following actions are now queued:

  • check copyright-compliance

There are 18 requests in the queue ahead of you.

Copy link
Contributor

@alexissantella alexissantella left a comment

Choose a reason for hiding this comment

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

re-approval for fixed unit tests. Looks good!

@FraserGreenroyd
Copy link
Contributor

@BHoMBot check ready-to-merge

@bhombot-ci
Copy link

bhombot-ci bot commented Jan 5, 2023

@FraserGreenroyd to confirm, the following actions are now queued:

  • check ready-to-merge

There are 1 requests in the queue ahead of you.

@FraserGreenroyd FraserGreenroyd merged commit bda3616 into develop Jan 5, 2023
@FraserGreenroyd FraserGreenroyd deleted the MEP_Engine-#2737-AddHVACHeatEquations branch January 5, 2023 18:41
@bhombot-ci bhombot-ci bot mentioned this pull request Mar 13, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type:feature New capability or enhancement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

MEP_Engine: Add HVAC Total, Sensible, Latent Heat Equations
3 participants