Fix faulty processing of tiered tech. choice coefficients in ecm_prep #527
Conversation
| if mskeys[4] not in consume_warn: | ||
| consume_warn.append(mskeys[4]) | ||
| verboseprint(opts.verbose, "WARNING: ECM '" + self.name + "' missing " | ||
| "valid consumer choice data for segment '" + str(mskeys) + | ||
| "'; using default choice data for refrigeration end use", "warning") |
There was a problem hiding this comment.
Nice verbose warning message! Still wondering if error vs warning is better, which we can discuss later.
|
|
||
| return choice_params | ||
|
|
||
| def fix_lgt_perf_vals(self, perf_in, mskeys): |
There was a problem hiding this comment.
In a future PR, we could introduce type hints to arguments. That would also help CI/linters catch errors quickly.
There was a problem hiding this comment.
A good idea more generally for Scout functions, let's leave this open.
scout/ecm_prep.py
Outdated
| # Ensure dictionary has end point data to draw from; if not, set segment | ||
| # coefficients to a default and notify user in verbose mode | ||
| if (tiered_flag and ( | ||
| any([x[1] in [0, "NA"] for x in choice_dict["typical"]["b1"].items()])) | ||
| or (not tiered_flag and | ||
| any([x[1] in [0, "NA"] for x in choice_dict["b1"].items()]))): |
There was a problem hiding this comment.
Does it make sense to check 0/NA for b2 as well?
There was a problem hiding this comment.
Good idea, I had assumed that if b1 was NA b2 would be too, but better to check both.
After considering further, I also made the check for a float, where non-floats would indicate a problem with the coefficient (zeros aren't necessarily a problem as the original implementation had assumed).
There was a problem hiding this comment.
The changes look great. I have added a few suggestions like type hints and extra checks for the b2 coefficient, but that can go to another PR if needed. Also, @jtlangevin mentioned he has tested this locally, and changes make sense.
The check now extends to both b1 and b2 coefficients and ensures the coefficient value is a float as expected, with anything other than a float resulting in the default set of coefficients being chosen.
jtlangevin
force-pushed
the
526-faulty-handling-of-tiered-tech-coefficients-in-ecm_prep
branch
from
d0243de to
0a66332
Compare
These had been missing, and are set to map cooking. Fossil other includes gas grills among other miscellaneous stuff like pool heaters, and backup generators.
This part of the code will generate an error when coefficients are tiered (as in most new AEO equipment mseg data). That error is caught and handled by assigning the mseg default coefficients (currently set to that of a typical refrigeration tech) which is not desirable for technologies with potentially unique choice considerations (e.g., HVAC tech.)
The fix includes: