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.

Sign up for GitHub

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

Jump to bottom

Implement new ice storage curve type #4442

Merged

Myoldmopar merged 7 commits into develop from 62494178-DetailedIceStoragePhase2

Oct 28, 2014

Contributor

RKStrand commented Aug 27, 2014

code work is now complete and this is ready for review and acceptance

Rick Strand added 2 commits

August 26, 2014 18:06


          initial changes for new CubicLinear enhancement

25da24f


          fixed bug in logic

RKStrand assigned Myoldmopar

Rick Strand added 2 commits

August 27, 2014 14:40


          changes to IDD needed to get new curve to work

596f46e


          New input file(s) testing new curve type

6da3eea

Member

Myoldmopar commented Aug 28, 2014

@RKStrand, if you click the "Files changed" tab above, and you browse through the code, you'll see that there are indentation issues. While most editors choose a tab size of 4 spaces, GitHub by default uses 8. This makes identifying tab/space issues easier on GitHub. I will fix the issues because it is very quick for me to do so. I'll let you know if I see any actual issues with the code besides this.

nrel-bot commented Sep 5, 2014

@Myoldmopar @RKStrand it has been 7 days since this pull request was last updated.

6 similar comments

nrel-bot commented Sep 12, 2014

@Myoldmopar @RKStrand it has been 7 days since this pull request was last updated.

nrel-bot commented Sep 19, 2014

@Myoldmopar @RKStrand it has been 7 days since this pull request was last updated.

nrel-bot commented Sep 26, 2014

@Myoldmopar @RKStrand it has been 7 days since this pull request was last updated.

nrel-bot commented Oct 3, 2014

@Myoldmopar @RKStrand it has been 7 days since this pull request was last updated.

nrel-bot commented Oct 10, 2014

@Myoldmopar @RKStrand it has been 7 days since this pull request was last updated.

nrel-bot commented Oct 17, 2014

@Myoldmopar @RKStrand it has been 7 days since this pull request was last updated.

Myoldmopar added 3 commits

October 23, 2014 10:58


          Fix indent issues; Add new idf to the CMakeLists file for testing

7a17800

[#62494178]


          Merge with develop, resolve conflicts

35f5102


          Update idf for 8.2 transition rules, runs fine now

4b3f2ec

Member

Myoldmopar commented Oct 23, 2014

@RKStrand I did some work on this branch:

Fixing indentation
Merging develop (including resolving conflicts)
Updating the idf with the 8.2 idd changes (sizing:system, plantloop, condenserloop)
Running the test file and looking at the results

Everything seems good to go on the code side. I emailed you requesting the docs since I can't seem to find them in the mix of feature emails. Once I review the docs, I think this should be merge-able.

Myoldmopar added a commit to NREL/EnergyPlusBuildSupport that referenced this pull request


          Added @RKStrand documentation for ice storage;

916f60a

Pull request: NREL/EnergyPlus#4442
[#62494178]

Myoldmopar added a commit that referenced this pull request


          Merge pull request #4442 from NREL/62494178-DetailedIceStoragePhase2

fc6343c

I'm merging this in.

@RKStrand Cleaned this up, built this, ran the test file, and everything looks good.  Looked over the docs and everything there is fine.  I'd agree that the engineering reference part is redundant, but thanks for adding those changes anyway.  

@kbenne @mjwitte This merge will be the first merge post-8.2.0 that changes inputs.  Doesn't require transition, but does affect input.  If we decide to do a re-release of 8.2.0, we can branch of off develop before this branch (@861708a6adadaedf2dbb3585863ec335a4b586a1) and apply the release-able changes.  

@RKStrand I'll move on to your other branches now, glad to get this process going for you.

Myoldmopar merged commit fc6343c into develop

Myoldmopar deleted the 62494178-DetailedIceStoragePhase2 branch

October 28, 2014 17:20

Member

Myoldmopar commented Oct 28, 2014

Oh, and I put your doc changes over here: https://github.com/NREL/EnergyPlusBuildSupport/tree/master/docs/src/mods/for_2015_03_release/RS-62494178-detailed-ice-storage-phase-2

Myoldmopar changed the title ~~62494178 detailed ice storage phase2~~ Implement new ice storage curve type

Myoldmopar added the NewFeature label

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels