Make generic update #227

spahrenk · 2025-03-12T19:53:53Z

Expands capability for refinement of HPWH models. Data structures for specifying model parameters to vary and figure-of-merit targets to meet have been expanded to allow greater flexibility. Currently, only COP polynomial coefficients are fully implemented as parameters, while only energy factors are fully implemented as figures-of-merit. Some simplification was introduced in this update, by inferring the condenser index from the hpwh object, rather than via manual entry. The lists of parameters and metrics are passed to the makeGeneric function in an object of type 'GenericOptions'.

Performance polynomial coefficient entries for Tier-3 and Tier-4 preset models were modified by inter/extrapolation, so that temperatures 50F, 67.46F, and 95F are used as references. This allows independent refinement of the energy factor at each of these temperatures. This method is demonstrated in a unit test added to measureMetrics.cpp.

Functions for refining COP coefficients for specific energy-factor tests E50, UEF, and E95, or the collection of all three, have been added.

Solution by the secant method was added as an alternative to least-squares refinement. The least-squares implementation here remains limited to two parameters and one metric. Secant can be used to refine one parameter to meet one metric.

…WHsim into make-generic-update

spahrenk · 2025-03-13T21:38:12Z

@nealkruis I haven't found any info about inlet temperatures for E50 or E95. I see that the test30-test95 in the test folder use the temps below:
test30: Ta = 0C, Tinlet = 5C
test50: Ta = 10C, Tinlet = 10C
test70: Ta = 20C, Tinlet = 20C
test95: Ta = 35C, Tinlet = 25C

spahrenk · 2025-03-14T19:11:15Z

@nealkruis Seems to do fine with secant method in unit test, keeping the COP slope fixed. Let me know what else is needed.

nealkruis

@spahrenk this is really good work on addressing the optimization problem!

As I familiarize myself with the DOE test standard, I think there are some ways to streamline the structure of the data and functions.

Finally, there are a number of areas where names/comments can be improved to make it easier for others to understand the code.

Let me know if you have any questions!

nealkruis · 2025-03-14T20:07:17Z

src/HPWH.cc

+    *standardTestOptions.outputStream << "\tFirst-Hour Rating:";
+    *standardTestOptions.outputStream << "\t\tVolume Drawn (L): " << firstHourRating.drawVolume_L
+                                      << "\n";
+    *standardTestOptions.outputStream
+        << "\t\tDesignation: " << FirstHourRating::sDesigMap[firstHourRating.desig] << "\n";
+


This kind of messaging should be handled through courier (send_info) or produced by the client code.

The output is directed to a text file here, when an output file path is specified, for the plotting utility. (I replaced that with JSON in a more recent version, and would like to pass the output over a websocket or similar, but haven't figured that out yet.) This was mostly a test feature, so I can definitely direct the output here to courier, rather than the console, when an output file path is not specified.

nealkruis · 2025-03-14T20:32:13Z

src/HPWHFitter.hh

+struct HPWH::GenericOptions
+{
+    std::vector<HPWH::Fitter::MeritInput*> meritInputs;
+    std::vector<HPWH::Fitter::ParamInput*> paramInputs;
+};


Please follow our guidelines and use comments/names to make it easier for me to understand what this struct contains and how it is used. I already mentioned that I don't like "GenericOptions" because it is very vague. Would "FittingParameters" be a more accurate and descriptive name?

These have been removed.

nealkruis · 2025-03-14T20:34:14Z

src/HPWHFitter.hh

+/**	Optimizer for varying model parameters to match metrics, used by
+ *  make_generic to implement a target UEF. The structure is fairly general, but
+ *  currently limited to one figure-of-merit (UEF) and two parameters (COP coeffs).
+ *  This could be expanded to include other FOMs, such as total energy in 24-h test.


Avoid using acronyms, especially when they aren't defined. FOM = figure-of-merit?

Sure, the structure needs some polishing, too.

nealkruis · 2025-03-14T20:59:34Z

src/HPWH.hh

+
+        bool overrideAmbientT = false;
+        double ambientT_C = 19.7;
+


I think we need to reconfigure CustomTestOptions and StandardTestOptions into a "TestConfiguration" that holds:

Ambient Temperature

Inlet Temperature

We can then create some static instances of this for the UEF, E50, and E90 tests.

Some options don't ever seem to change, and I don't think we need to include them as something people calling the function can change:

Setpoint Temperature is always 51.7C

Change Setpoint? is always false

Number of Test Thermocouples is always 6

We may still want a separate set of settings that control test output, but really all the file output options should be handled by client code (not in the library itself).

There might be good reasons for the structures that I'm missing, but they don't feel intuitive to me.

nealkruis · 2025-03-14T21:01:17Z

src/HPWH.hh

+    void makeGenericUEF(double targetEF, double ambientT_C = 19.7);
+    void makeGenericE50(double targetEF) { return makeGenericUEF(targetEF, 10.); }
+    void makeGenericE95(double targetEF) { return makeGenericUEF(targetEF, 35.); }


per comment above, I think we can change these so that they use the static "TestConfiguration"s I described above.

nealkruis · 2025-03-14T21:10:41Z

src/HPWH.cc

+    const double ambientT_C = (customTestOptions.overrideAmbientT)
+                                  ? customTestOptions.ambientT_C
+                                  : 19.7; // EERE-2019-BT-TP-0032-0058, p. 40435


Pull from "TestConfiguration" instead (see comment below).

nealkruis · 2025-03-14T21:10:48Z

src/HPWH.cc

+                                  ? customTestOptions.ambientT_C
+                                  : 19.7; // EERE-2019-BT-TP-0032-0058, p. 40435
+    const double externalT_C = ambientT_C;
+    double inletT_C = findInletT_C(ambientT_C);


Pull from "TestConfiguration" instead (see comment below).

nealkruis · 2025-03-14T21:11:21Z

src/HPWH.cc

+    const double ambientT_C = (customTestOptions.overrideAmbientT)
+                                  ? customTestOptions.ambientT_C
+                                  : 19.7; // EERE-2019-BT-TP-0032-0058, p. 40435


Pull from "TestConfiguration" instead (see comment below).

nealkruis · 2025-03-14T21:11:26Z

src/HPWH.cc

+                                  ? customTestOptions.ambientT_C
+                                  : 19.7; // EERE-2019-BT-TP-0032-0058, p. 40435
+    const double externalT_C = ambientT_C;
+    double inletT_C = findInletT_C(ambientT_C);


Pull from "TestConfiguration" instead (see comment below).

nealkruis · 2025-03-14T21:13:59Z

src/HPWH.cc

+    const double ambientT_C = (customTestOptions.overrideAmbientT)
+                                  ? customTestOptions.ambientT_C
+                                  : 19.7; // EERE-2019-BT-TP-0032-0058, p. 40435
+    const double externalT_C = ambientT_C;
+    double inletT_C = findInletT_C(ambientT_C);


The First Hour Rating is always performed at 67.5F ambient and 58F inlet temperatures. It should be run independently of the 24 hour test, and the 24 hour test should take the first hour rating draw designation as an input.

This is very useful, thx.

@nealkruis
-Changed parameter names.
-Defined TestConfiguration. Declared static instances.
-Improved comments.
-Improved fit initialization.
-Use UEF temps for first-hour rating.
-Reported output with courier.

nealkruis · 2025-03-24T15:57:24Z

src/HPWH.cc

+    double maxAllowedSetpointT_C;
+    std::string why;
+    if (isNewSetpointPossible(HPWH::testSetpointT_C, maxAllowedSetpointT_C, why, UNITS_C))
    {
-        setSetpoint(testOptions.setpointT_C, UNITS_C);
+        setSetpoint(HPWH::testSetpointT_C);
+    }
+    else
+    {
+        if (isNewSetpointPossible(maxAllowedSetpointT_C, maxAllowedSetpointT_C, why, UNITS_C))
+            setSetpoint(maxAllowedSetpointT_C);


I like that you're leveraging an existing function here, though there are some specters here that make it hard to understand what is happening. The design of isNewSetpointPossible isn't great, but it can be improved in a separate branch/PR.

We just need to assign a setpoint somewhere. Rewrite the section without using the functions?

spahrenk added 11 commits March 7, 2025 14:22

Separate fitting into struct.

6ef6df8

Initialize outputStream.

744163e

Add comments.

11fedfa

Add cop constraints.

02cfb43

Enable secant fitting.

90dc916

Fix COP constraint.

53af44f

Add multivariate framework.

a35674c

Improve reporting.

bb2822c

Improve results tracking.

fdc4b69

Add makeGeneric tests.

cb2d7a9

Add Fitter file; separate methods.

1fd6329

spahrenk self-assigned this Mar 12, 2025

spahrenk marked this pull request as draft March 12, 2025 19:54

spahrenk added the enhancement label Mar 12, 2025

spahrenk requested a review from nealkruis March 12, 2025 20:00

spahrenk and others added 9 commits March 12, 2025 14:42

Change threshold.

8e617f7

Move Fitter to separate files.

9747ef5

Define fncs for E50, UEF, E95/

6fa54e3

Share courier among params,merits.

cb7cbed

Cast as int.

6750868

Remove assign.

99b6309

Improve comments.

2e3b79e

Merge branch 'make-generic-update' of https://github.com/bigladder/HP…

a9d6168

…WHsim into make-generic-update

Use fabs, not abs.

d014689

spahrenk marked this pull request as ready for review March 13, 2025 20:55

Expand comments.

2628059

Organize; remove test code; use secant.

c73dc58

nealkruis requested changes Mar 14, 2025

View reviewed changes

spahrenk and others added 25 commits March 29, 2025 16:55

Consolidate fncs.

cb91d62

Add test, getAmbientT_index fnc.

d9db36b

Format.

650305a

Remove show parameters.

2e74d70

Change to Designation.

c7cc992

Spell out Coefficient.

1a198e1

Expand names.

8150ad2

Expand comments.

4e8fd55

Change to prepare.

3a3826f

Format.

e781fca

Spell out average.

a974e12

Remove unused var.

908d88d

Remove s prefix on string.

bed8508

Expand to Performance.

9fe2d7a

Change to DesignationMap.

df9ac91

Remove s- in string.

243c602

Change to presetOrFile.

aad0dea

Simplify and opdate least-squares.

d42c5db

Format.

d26f21e

Return value from evaluate().

f76360c

Chagne var name, comment.

2288fa5

Fix type, comment.

779ae54

Format.

f92dd3e

Improve error/warning messages.

5db9f57

Apply clang-format.

dd09e5a

spahrenk linked an issue Apr 3, 2025 that may be closed by this pull request

Generate data-model code in vendor using repo in build #228

Closed

spahrenk removed a link to an issue Apr 3, 2025

Generate data-model code in vendor using repo in build #228

Closed

nealkruis approved these changes Apr 16, 2025

View reviewed changes

nealkruis merged commit e4c8d68 into main Apr 16, 2025
7 checks passed

nealkruis deleted the make-generic-update branch April 16, 2025 15:18

Make generic update #227

Make generic update #227

Uh oh!

Conversation

spahrenk commented Mar 12, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

spahrenk commented Mar 13, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

spahrenk commented Mar 14, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

nealkruis left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

spahrenk Mar 14, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

spahrenk Apr 16, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

spahrenk commented Mar 12, 2025 •

edited

Loading

spahrenk commented Mar 13, 2025 •

edited

Loading

spahrenk commented Mar 14, 2025 •

edited

Loading

spahrenk Mar 14, 2025 •

edited

Loading

spahrenk Apr 16, 2025 •

edited

Loading