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

Lithium Bromide-Water Heat Pump Media and Components #14

Open
wants to merge 39 commits into
base: devel
Choose a base branch
from

Conversation

puncan
Copy link
Collaborator

@puncan puncan commented Jun 16, 2021

The LiBr-H2O media package has been added. A very simple absorption cycle with the relevant components is included as well. I still plan on added chemical beds but the most recent update is giving me issues. These issues are included in the NASA_Glenn media packages, CaO and Ca(OH)2.

@klfrick2 klfrick2 requested a review from mikkdm June 16, 2021 17:29
Copy link
Collaborator

@mikkdm mikkdm left a comment

Choose a reason for hiding this comment

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

Submission of fewer files is needed, some of these are minor spacing changes and not direct edits. Also, files should be added individually and separately rather than as inside of existing files (see Models/NHES/Fluid/HeatExchangers/Generic_HXs/package.mo). Told Paul how to save separately and only add specific files to commits.

@puncan puncan requested a review from mikkdm June 23, 2021 18:00
@puncan
Copy link
Collaborator Author

puncan commented Jun 23, 2021

The file structure was updates so packages and models are added as individual files now. I was able to remove some of the accidental (likely insignificant) changes to packages I am not involved with but not all. I know how to avoid this in future commits and pull requests.

Copy link
Collaborator

@mikkdm mikkdm left a comment

Choose a reason for hiding this comment

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

We shall set up some time to help with this as much as possible.
Remove .bak-mo file extensions. These are dymola backup files and shouldn't be necessary, they're either things you deleted or temporary duplicates.
Some other files that do not appear to be from your primary work are in the commit as well.

@mikkdm mikkdm requested a review from klfrick2 June 28, 2021 22:17
Removing bak-mo files
@@ -1,9 +0,0 @@
within NHES.Fluid;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can you check to make sure this should be deleted?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It should not be deleted. I have added a heat exchanger I would like to keep and an effectiveness function for heat exchangers.

@@ -1,5 +0,0 @@
Examples
Copy link
Collaborator

Choose a reason for hiding this comment

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

What's happening with this package.order file? I'm not sure why it's being deleted effectively instead of being edited or something.

@@ -1,1229 +0,0 @@
within NHES.Systems.BalanceOfPlant.StagebyStageTurbineSecondary;
Copy link
Collaborator

Choose a reason for hiding this comment

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

This file shouldn't be changed. Github isn't letting me delete it.

@@ -1,7689 +0,0 @@
within NHES.Systems.EnergyStorage.ThermoclineHeatStorage;
Copy link
Collaborator

Choose a reason for hiding this comment

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

This file shouldn't be changed. Github isn't letting me delete it.

@@ -1,1391 +0,0 @@
within NHES.Systems.BalanceOfPlant.StagebyStageTurbineSecondary;
Copy link
Collaborator

Choose a reason for hiding this comment

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

This file shouldn't be changed. Github isn't letting me delete it.

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.

4 participants