pnnl / tesp

Other
39 stars 37 forks source link

Fixes to SGIP1 example, standalone housing generator function #48

Closed temcdrm closed 3 years ago

temcdrm commented 3 years ago

Fixes for:

Also has the preliminary version of a separate function that writes houses for a specific node, for CEF2 project.

trevorhardy commented 3 years ago

@temcdrm: Since I've installed TESP via the installer, how do I get these changes into my installation to test?

temcdrm commented 3 years ago

I think you can do that with <= 10 hand-edits from the last two commits:

temcdrm commented 3 years ago

you don't need the change to api.py in the last commit

trevorhardy commented 3 years ago

I tested SGIP1c with the changes made and using the default plots.py I'm still seeing a flat price in the EnergyPlus plots. Is that working for you, @temcdrm?

temcdrm commented 3 years ago

Yes, it's working for me on SGIP1b. This is the key change: https://github.com/pnnl/tesp/pull/48/commits/17a286fe06770e2dd305c27598aad01bef74dc5b

There should be no difference with SGIP1c or the others, because they share the same yaml file.

trevorhardy commented 3 years ago

I noticed that all the .glm files reference SGIP1b_FNCS_Config.txt; I'm assuming they should all be referencing their own config file?

temcdrm commented 3 years ago

There are only two variants of FNCS_Config.txt, one for A and one for B. For C, D, E the differences are in DER, not controllers.

trevorhardy commented 3 years ago

@temcdrm, let me know when you're ready to consider this PR complete and we'll review and merge it. From the original commits I've been able to verify the fixes you've put in place so I think we're good so far.

temcdrm commented 3 years ago

This is ready to review and merge

laurmarinovici commented 3 years ago

Is this latest code review, for SGIP1 example running on a FNCS-based TESP or HELICS-based? I currently do not have a FNCS-based one, and I am not up to date with what works and what not version-wise.

temcdrm commented 3 years ago

Probably FNCS, but the configurable version should also work with HELICS. See examples/comm/make_comm_base.py, where it configures from SGIP1c.json

temcdrm commented 3 years ago

Trevor & Laurentiu - should I go ahead and merge this?

laurmarinovici commented 3 years ago

@temcdrm Is there a latest version of docker I could use? I tried running the prepare_cases.py in sgip example inside my docker container, but I do not have the correct environment variables and paths, which results in error; see:

Traceback (most recent call last):
  File "prepare_cases.py", line 5, in <module>
    tmy_file = os.getenv('TESP_INSTALL') + '/share/support/weather/AZ-Tucson_International_Ap.tmy3'
TypeError: unsupported operand type(s) for +: 'NoneType' and 'str'

On Docker hub I could only find docker pull temcderm/tesp_core:1.0.1, which seems to be from 7 months ago, and up to path 1 compared to patch 2 in the release.

I have also tried to install release v1.0.2 on an ubuntu 20.4 docker, but I got this error

Installing
 0% ______________ 50% ______________ 100%

Error: There has been an error.
Error running apt-get -y install coinor-cbc: E: Unable to locate package 
coinor-cbc

However, I looked through the code file by file, and everything looks legit and I am sure it works in the right environment. So, I am fine with the merge.

temcdrm commented 3 years ago

I've updated the two Dockerfiles for 1.0.2. One change was caused by removal of apt-utils from the package repository; not sure why, and the first docker build throws a warning but runs to completion. I also had to run apt update twice within the image for tesp_foundation; not sure why.