Issue 304 - feature(process-cooling): return unadjusted chiller efficiency profilee curve derived from ARI#305
Issue 304 - feature(process-cooling): return unadjusted chiller efficiency profilee curve derived from ARI#305nbintertech wants to merge 2 commits intodevelopfrom
Conversation
…e curve derived from ARI
There was a problem hiding this comment.
Pull request overview
This PR adds an additional output to Process Cooling chiller results: an unadjusted ARI-derived efficiency profile curve (by load bin) intended for MEASUR Performance Profile rendering.
Changes:
- Extend
ProcessCooling::ChillerOutputto includeariEfficiencyProfile(11 load bins per chiller) and return it fromcalculateChillerEnergy(). - Compute and store the ARI efficiency profile curve via a new
buildChillerEfficiencyARIProfile()step duringProcessCoolinginitialization. - Expose the new output through the WASM bindings and add/update C++ + WASM tests to validate the curve values.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
src/processCooling/process_cooling.cpp |
Stores ARI coefficients, builds and returns the unadjusted ARI efficiency profile curve. |
include/processCooling/process_cooling.h |
Extends ChillerOutput with ariEfficiencyProfile and adds internal storage for ARI coefficients/profile. |
bindings-wasm/processCooling/process_cooling.cpp |
Exposes ariEfficiencyProfile on the WASM ChillerOutput binding. |
tests/cpp/processCooling/process_cooling.unit.cpp |
Adds assertions validating ariEfficiencyProfile size/values in unit tests. |
tests/wasm-mocha/processCooling/wasm_process_cooling.test.js |
Adds assertions validating ariEfficiencyProfile via WASM/mocha tests. |
| vector<int> capacityIndex(31, 0); | ||
| vector<vector<double>> efficiencyData( | ||
| numChillers, vector<double>(7, 0)); // 7 columns for each chiller. Dummy array for chiller values | ||
| vector<int> capacityIndex(31, 0); |
There was a problem hiding this comment.
capacityIndex is initialized with a fixed size of 31 but is indexed by chiller index c (0..numChillers-1). If numChillers > 31 this will read/write out of bounds. Size this vector to numChillers (or otherwise avoid indexing it by c) to prevent memory corruption.
| vector<int> capacityIndex(31, 0); | |
| vector<int> capacityIndex(numChillers, 0); |
There was a problem hiding this comment.
Ignoring, see CWSAT_ported_improvements.md
| int idx = capacityIndex[c]; | ||
| if (coolingType == CoolingSystemType::Water) { |
There was a problem hiding this comment.
int idx = capacityIndex[c]; will always yield 0 with the current code because capacityIndex is only initialized to zeros and never set based on chiller capacity/tonnage (the earlier logic only clamps values >N, but none are ever assigned). This means ARI coefficients will always be taken from the first row of the lookup tables; populate capacityIndex[c] from the relevant Array*Data* table based on the chiller’s tonnage before applying the existing clamping rules.
There was a problem hiding this comment.
Ignoring, see CWSAT_ported_improvements.md
…put, add doc for potential ported code changes
|
Ignoring LLM concerns on code outside this PR. Not really clear how the requested changes will change desired output. I'm hesitant to change the ported C++ code without doing some more digging in the VB project or relying on Omer. I have what I need to start getting the performance profile implemented in MEASUR, but will have to come back to this. |
#304
I will leave this in draft until I can get clarification from experts on how it is I'm supposed to verify we're outputting the correct curve information.