Control Volume 1D Scaler by dallan-keylogic · Pull Request #1673 · IDAES/idaes-pse

dallan-keylogic · 2025-09-26T20:13:03Z

Fixes

Allows submodel scalers to be called on BlockData directly, not just IndexedBlocks

Summary/Motivation:

Adds a scaler for the ControlVolume1D.

Changes proposed in this PR:

Adds scaler for ControlVolume1D
Adds public flow_direction property to allow public access to the _flow_direction attribute.

Legal Acknowledgement

By contributing to this software project, I agree to the following terms and conditions for my contribution:

I agree my contributions are submitted under the license terms described in the LICENSE.txt file at the top level of this directory.
I represent I am authorized to make the contributions and grant the license. If my employer has rights to intellectual property that includes these contributions, I represent that I have received permission to make contributions and grant the required license on behalf of that employer.

codecov · 2025-09-26T21:23:07Z

Codecov Report

❌ Patch coverage is 99.15966% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 77.15%. Comparing base (ca3c1fd) to head (321fae0).
⚠️ Report is 27 commits behind head on scaling_toolbox.

Files with missing lines	Patch %	Lines
idaes/core/base/control_volume_base.py	97.61%	0 Missing and 1 partial ⚠️

Additional details and impacted files

@@                 Coverage Diff                 @@
##           scaling_toolbox    #1673      +/-   ##
===================================================
+ Coverage            77.09%   77.15%   +0.05%     
===================================================
  Files                  395      395              
  Lines                63182    63278      +96     
  Branches             10380    10418      +38     
===================================================
+ Hits                 48713    48824     +111     
+ Misses               12021    12012       -9     
+ Partials              2448     2442       -6

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

MarcusHolly · 2025-10-14T14:49:48Z

idaes/core/base/control_volume1d.py

+        # if hasattr(self, "elemental_flow_term"):
+        #     for (t, x, e), v in self.elemental_flow_term.items():
+        #         flow_basis = self.properties[t, x].get_material_flow_basis()
+
+        #         sf = iscale.min_scaling_factor(
+        #             [
+        #                 self.properties[t, x].get_material_density_terms(p, j)
+        #                 for (p, j) in phase_component_set
+        #             ],
+        #             default=1,
+        #             warning=True,
+        #         )
+        #         if flow_basis == MaterialFlowBasis.molar:
+        #             sf *= 1
+        #         elif flow_basis == MaterialFlowBasis.mass:
+        #             # MW scaling factor is the inverse of its value
+        #             sf *= value(self.properties[t, x].mw_comp[j])
+
+        #         iscale.set_scaling_factor(v, sf)
+
+        # if hasattr(self, "elemental_flow_dx"):
+        #     for (t, x, e), v in self.elemental_flow_dx.items():
+        #         if iscale.get_scaling_factor(v) is None:
+        #             # As domain is normalized, scale should be equal to flow
+        #             sf = iscale.get_scaling_factor(self.elemental_flow_term[t, x, e])
+        #             iscale.set_scaling_factor(v, sf)


Will this be implemented in a separate PR?

Yes. Since the Gibbs reactor is the only unit model using element balances, I was hoping that @agarciadiego would take care of it when he revisited the Gibbs reactor scaling. If not, I'll take care of it.

However, many unit models depend on the ControlVolume1D, so I'd rather make this scaler available now and then revisit element balance scaling later.

MarcusHolly · 2025-10-14T15:00:02Z

idaes/core/base/control_volume1d.py

+
+        if hasattr(model, "_flow_terms"):  # pylint: disable=protected-access
+            for idx, v in model._flow_terms.items():
+                self.scale_variable_by_definition_constraint(


scale_variable_by_definition_constraint is a new capability right? I don't see it in main, but I also don't see it being added to custom_scaler_base.py in this PR. As in, I see that the version of custom_scaler_base.py in this PR has this new functionality, but it's not appearing as one of the changed lines. Why is this happening?

It lives in the scaling_toolbox feature branch right here. That's where we've been merging all the scaling PRs, and that's where this PR is pointed.

MarcusHolly · 2025-10-14T15:32:09Z

idaes/core/base/tests/test_control_volume_1d_scaler_object.py

+    scaler_obj.default_scaling_factors["area"] = 1 / 83
+    scaler_obj.default_scaling_factors["length"] = 1 / 599


These are marked as having a required user input. What happens if a user does not supply scaling factors for these? Does an error or warning pop up. If so, is this being tested somewhere?

An error pops up.

I don't think the error is being tested specifically for the ControlVolume1D, but the general feature is being tested here.

As long as it's being tested somewhere, I think it's fine

MarcusHolly

LGTM

Ryan-Hughes-8

This looks good to me. I didn't get a chance to test this locally on any examples, but I assume the incoming unit model scalers will be additional stress tests.

Ryan-Hughes-8 · 2025-10-14T18:45:29Z

idaes/core/base/control_volume1d.py

+        self.propagate_state_scaling(
+            target_state=model.properties,
+            source_state=model.properties[x_in],
+            overwrite=overwrite,
+        )
+        self.call_submodel_scaler_method(
+            submodel=model.properties,
+            submodel_scalers=submodel_scalers,
+            method="variable_scaling_routine",
+            overwrite=overwrite,
+        )


When you call these methods on an indexed component, will it iterate through the indices and apply to each of them?

Yes it will. The original method wouldn't even let you call it on BlockData children of an IndexedBlock, but this PR adds a change that allows you to call it both on an IndexedBlock and BlockData.

aostace01

The PR looks great!

A question for perhaps a future PR: are there user-facing docs that need updating to explain how to use this scaler and what inputs are required?

dallan-keylogic · 2025-10-15T14:52:05Z

The PR looks great!

A question for perhaps a future PR: are there user-facing docs that need updating to explain how to use this scaler and what inputs are required?

Yes, we need to update the documentation soon. At some point, I am hoping to create a method that automatically tells you which scaling factors are expected by a Scaler object but making sure all the scalers exist is the first priority. Also, I'd like to see them get used in the projects to see if we need to make any changes to the interface to make them easier to use. After they reach their final form, we can update the documentation.

* additional test for propagating scaling factors within a block * Control Volume 1D scaler * phase material balance cv0d * test for submodel scaler for blockdata * Tests for flow direction attribute * black and typo * remove unused variables * protected property error message depends on python version * test no override * test propagate scaling factors * additional test coverage

dallan-keylogic added 6 commits September 25, 2025 10:48

additional test for propagating scaling factors within a block

82a1e84

Control Volume 1D scaler

d27e0f4

phase material balance cv0d

3483fb3

test for submodel scaler for blockdata

557eed5

Merge branch 'scaling_toolbox' into control_volume_1D_scaler

96de1a0

Tests for flow direction attribute

e5582d4

dallan-keylogic requested a review from agarciadiego as a code owner September 26, 2025 20:13

dallan-keylogic added 3 commits September 26, 2025 16:15

black and typo

929f3a6

remove unused variables

dd900f6

protected property error message depends on python version

be2d0b1

dallan-keylogic changed the title ~~Control volume 1D Scaler~~ Control Volume 1D Scaler Sep 29, 2025

dallan-keylogic added 3 commits September 29, 2025 15:27

test no override

05b4abd

test propagate scaling factors

0d86465

additional test coverage

321fae0

dallan-keylogic mentioned this pull request Oct 1, 2025

Mixer Scaler #1676

Merged

dallan-keylogic requested review from Ryan-Hughes-8 and aostace01 October 6, 2025 16:06

MarcusHolly reviewed Oct 14, 2025

View reviewed changes

MarcusHolly approved these changes Oct 14, 2025

View reviewed changes

Ryan-Hughes-8 approved these changes Oct 14, 2025

View reviewed changes

aostace01 approved these changes Oct 15, 2025

View reviewed changes

dallan-keylogic merged commit 0a8eed3 into IDAES:scaling_toolbox Oct 15, 2025
109 checks passed

		scaler_obj.default_scaling_factors["area"] = 1 / 83
		scaler_obj.default_scaling_factors["length"] = 1 / 599

Conversation

dallan-keylogic commented Sep 26, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Fixes

Summary/Motivation:

Changes proposed in this PR:

Legal Acknowledgement

Uh oh!

codecov bot commented Sep 26, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

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!

MarcusHolly left a comment

Choose a reason for hiding this comment

Uh oh!

Ryan-Hughes-8 left a comment

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!

aostace01 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

dallan-keylogic commented Oct 15, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

dallan-keylogic commented Sep 26, 2025 •

edited

Loading

codecov bot commented Sep 26, 2025 •

edited

Loading