Update for pysam hybrids by dguittet · Pull Request #1091 · NatLabRockies/ssc

dguittet · 2023-11-16T18:26:28Z

The input technologies to cmod_hybrid require some financial variables that aren't listed as required, and aren't checked by the cmod. In PySAM, this means if the user doesn't have these extra financial variables, the cmod will just have a hard exit. Fix this by making a vtab_hybrid_tech_om_inputs that isn't added to any of the compute modules, but is added dynamically by cmod_hybrid to check that all these variables are present.

Other changes to cmod_hybrid:

Move check of required input input_tables to beginning of exec
Use inflation_rate and analysis_period from the "Hybrid" input_table which includes the financial cmods. The code originally requires each cmod to have its own inflation_rate and analysis_period which is problematic because not all technology cmods have these as inputs, and it seems to make it possible to have different values. However, this requires there to be a financial cmod to the hybrid cmod, and this is currently not enforced. Should a financial model be required for running a hybrid cmod?
Not all technology cmods have system_use_lifetime_output, such as windpower, so instead of adding a new interface variable to these cmods, I made cmod_hybrid assume False unless the variable is available and True.

Happy to discuss concerns about any of these proposed changes.

sjanzou · 2023-11-26T11:37:26Z

The input technologies to cmod_hybrid require some financial variables that aren't listed as required, and aren't checked by the cmod. In PySAM, this means if the user doesn't have these extra financial variables, the cmod will just have a hard exit. Fix this by making a vtab_hybrid_tech_om_inputs that isn't added to any of the compute modules, but is added dynamically by cmod_hybrid to check that all these variables are present.

Other changes to cmod_hybrid:

Move check of required input input_tables to beginning of exec

Use inflation_rate and analysis_period from the "Hybrid" input_table which includes the financial cmods. The code originally requires each cmod to have its own inflation_rate and analysis_period which is problematic because not all technology cmods have these as inputs, and it seems to make it possible to have different values. However, this requires there to be a financial cmod to the hybrid cmod, and this is currently not enforced. Should a financial model be required for running a hybrid cmod?

Not all technology cmods have system_use_lifetime_output, such as windpower, so instead of adding a new interface variable to these cmods, I made cmod_hybrid assume False unless the variable is available and True.

Happy to discuss concerns about any of these proposed changes.

A financial compute module is currently required to run a hybrid compute module. We can revisit in the future. Required for the o and m and installation costs for each hybrid component separately in a hybrid system. The technology compute modules do not require the costs to run.

The battery and fuel cell compute modules are run in lifetime mode regardless of the generator compute modules lifetime mode settings.

sjanzou · 2024-02-16T06:58:34Z

@dguittet, are you working on this branch or a different one for patch 1? Thanks!

dguittet · 2024-02-16T18:43:59Z

@sjanzou Yes, this is the one still.

sjanzou

One test failing on Windows - all others are passing!
[ FAILED ] CmodHybridTest.GenericPVWattsWindFuelCellBatteryHybrid_SingleOwner

sjanzou · 2024-02-20T09:00:16Z

With latest changes, all ssc tests passing but all Hybrid FuelCell configurations are still failing to run per NatLabRockies/SAM#1584

dguittet · 2024-02-21T13:29:29Z

+                    if (compute_module == "generic_system")
+                        degrad = input.as_array("generic_degradation", &count_degrad);
+                    else
+                        degrad = input.as_array("degradation", &count_degrad);


@sjanzou The reason the SAM tests are failing to run the Generic PVWatts Wind FuelCell Battery Hybrid system is because of these lines. The SAM ui page for the Generic System's AC degradation uses "degradation" for both Hybrid and regular Generic System cases. However, the SSC input to generic system is "generic_degradation". It seems like the generic_degradation is used within cmod_generic_system whereas degradation is used in the hybrid and financial compute modules?

Plot showing no "generic_degradation" in use

Anyways, is 'generic_degradation' used at all? In the Hybrid modules, we can just use "degradation" for all technologies that aren't fuelcell since that once has it's own "fuelcell_degradation". But then what if a user has different values for "generic_degradation" and "degradation"?

generic_degradation is input to both cmod_generic_system and cmod_mhk_wave and is in the UI in Degradation AC Lifetime form as follows:

So, is used throughout the generic system.

Also, "degradation" is only used for non-lifetime simulation like PVWatts, pvsamv1 applies degradation outside of cmod_hybrid and is not used in cmod_hybrid (true for all generators with system_use_lifetime_output non-zero):

The code you showed for cmod_hybrid appears to be out of date... generic_degradation is not used in cmod_hybrid - when did you last merge patch into pysam_hybrids? Did it merge the latest from patch?

The latest patch branch does not have the lines 217 through 220 that you showed
https://github.com/NREL/ssc/blob/patch/ssc/cmod_hybrid.cpp

dguittet · 2024-02-27T23:11:18Z

+    compute_module* cmod = static_cast<compute_module*>(p_mod);
+    if (!p_mod)
+        return 0;
+


add comment for why 35

dguittet · 2024-02-27T23:12:49Z

 	add_var_info(vtab_fuelcell_output);
 	add_var_info(vtab_technology_outputs);
-    add_var_info(vtab_hybrid_tech_om);
+    add_var_info(vtab_hybrid_tech_om_outputs);


move into hybridize?

dguittet · 2024-02-27T23:14:18Z

+            var_data* financial_compute_modules = input_table->table.lookup("hybrid");
+            int analysisPeriod = (int)financial_compute_modules->table.lookup("analysis_period")->num;
+            ssc_number_t inflation_rate = financial_compute_modules->table.lookup("inflation_rate")->num * 0.01;
+            ssc_number_t sales_tax_rate = financial_compute_modules->table.lookup("sales_tax_rate")->num * 0.01;


Comment out and check tests

dguittet · 2024-02-27T23:15:26Z

-                    degrad = input.as_array("degradation", &count_degrad);
+                    ssc_number_t* degrad = input.as_array("degradation", &count_degrad);
+                    if (compute_module == "generic_system")
+                        input.assign("generic_degradation", *input.lookup("degradation"));


@sjanzou Can check this? I think generic_system doesn't run lifetime, so degradation is used in cmod_hybrid and financials to extend out to lifetime

dguittet · 2024-02-27T23:17:59Z

                }
                // optional fossil fuel costs
-                if (compute_module_inputs->table.lookup("om_fuel_cost")) {
+                if (compute_module_inputs->table.lookup("system_heat_rate")) {


trying to prevent seg fault of accessing the following variables

dguittet · 2024-02-27T23:18:58Z

+                if (financial_compute_modules->table.is_assigned("ppa_multiplier_model"))
+                compute_module_inputs->table.assign("ppa_multiplier_model", *financial_compute_modules->table.lookup("ppa_multiplier_model"));
+                if (financial_compute_modules->table.is_assigned("ppa_price_input"))
+                compute_module_inputs->table.assign("ppa_price_input", *financial_compute_modules->table.lookup("ppa_price_input"));


reformat, and hopefully this makes sense and aligns with UI logic

Code generation for hybrids limited to only compute module variables.

Run all ssc tests

dguittet · 2024-03-04T18:21:43Z

@sjanzou Ready for review now that tests are updated from code generator and passing

sjanzou · 2024-03-05T10:54:12Z

Have issues with changing results and why the PVWattsv8WindBatterySingleOwner ssc test is three times slower than the patch version of the same test. The Host Developer is about the same.

The code generation results differ for the same tests in both pysam_hybrids on left and patch branch on right

… configurations

sjanzou

Working on windows and results matching with patch branches ssc and SAM.

Great work!

update for pysam hybrids

cab33c4

dguittet requested a review from sjanzou November 16, 2023 18:26

fix inflation rate

11bcf67

dguittet and others added 7 commits December 4, 2023 16:01

fix to work on windows

200a973

Merge branch 'develop' into pysam_hybrids

aa0117d

update cmod_hybrid

a68f413

remove print statement

3750ecf

Merge branch 'develop' into pysam_hybrids

a3ea9c4

fix test/CMakeLists.txt for Debug

6de6421

Merge branch 'develop' into pysam_hybrids

ac1f661

brtietz changed the base branch from develop to patch December 13, 2023 16:36

dguittet and others added 2 commits December 13, 2023 15:03

Merge branch 'develop' into pysam_hybrids

cb7f0da

adjustment factors default to 0

6f87cf3

Merge branch 'patch' into pysam_hybrids

c624a92

sjanzou requested changes Feb 19, 2024

View reviewed changes

fix hybrid test on windows

43f79e1

dguittet added 2 commits February 20, 2024 15:32

fix degradation input for hybrids

c06c5eb

HybridTech degradation as optional

5da35c7

dguittet commented Feb 21, 2024

View reviewed changes

dguittet added 2 commits February 21, 2024 13:50

fix degradation for generic_system in hybrids

ab14bc0

add HybridCosts to interface

a395353

dguittet commented Feb 27, 2024

View reviewed changes

dguittet and others added 15 commits February 29, 2024 07:39

sales_tax_rate optional

4e55a3a

add checks for existing var_info

ef83112

Merge branch 'patch' into pysam_hybrids

7e755c0

Merge branch 'patch' into SAM_1515

b3d6c94

JSON code generator test now fails with patch branch.

de1a3a0

Code generation for hybrids limited to only compute module variables.

Merge branch 'patch' into pysam_hybrids

0f54bce

Merge branch 'pysam_hybrids' into SAM_1515

6afd798

Update hybrid code generation test

419d0d9

ssc hybridized code generated JSON file from SAM

8a638e6

Run all ssc tests

Merge branch 'SAM_1515' into pysam_hybrids

bf3e5e7

remove comment

be7e768

update cmod_hybrid_test with code generation results

d39d19e

update all cmod_hybrids_test to use code generator

39f3522

fix codegenerator for "hybrid" inputs

f654a9f

update cmod_hybrid_Test input jsons

e64326b

dguittet and others added 2 commits March 5, 2024 14:28

fix battery capacity in hybrids

e7ec71e

Fix failing CmodHybridTest and align with patch branch of SAM default…

c556803

… configurations

sjanzou self-requested a review March 6, 2024 09:14

sjanzou approved these changes Mar 6, 2024

View reviewed changes

sjanzou merged commit 222bde6 into patch Mar 6, 2024

Conversation

dguittet commented Nov 16, 2023

Uh oh!

sjanzou commented Nov 26, 2023

Uh oh!

sjanzou commented Feb 16, 2024

Uh oh!

dguittet commented Feb 16, 2024

Uh oh!

sjanzou left a comment

Choose a reason for hiding this comment

Uh oh!

sjanzou commented Feb 20, 2024

Uh oh!

dguittet Feb 21, 2024

Choose a reason for hiding this comment

Uh oh!

sjanzou Feb 22, 2024

Choose a reason for hiding this comment

Uh oh!

dguittet Feb 27, 2024

Choose a reason for hiding this comment

Uh oh!

dguittet Feb 27, 2024

Choose a reason for hiding this comment

Uh oh!

dguittet Feb 27, 2024

Choose a reason for hiding this comment

Uh oh!

dguittet Feb 27, 2024 • 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!

dguittet Feb 27, 2024

Choose a reason for hiding this comment

Uh oh!

dguittet Feb 27, 2024

Choose a reason for hiding this comment

Uh oh!

dguittet commented Mar 4, 2024

Uh oh!

sjanzou commented Mar 5, 2024

Uh oh!

sjanzou left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

dguittet Feb 27, 2024 •

edited

Loading