-
Notifications
You must be signed in to change notification settings - Fork 234
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Minor improvements to scaling API #1507
Changes from all commits
528a4f7
0faabba
47d9d73
47beac6
c39f256
04f1463
3ce08dd
4aa1405
7353472
e88bd83
d5135f4
9ccc8b4
9423501
6754964
f6079ca
ad475fd
dda16b3
bc8e411
4273e24
542914a
6578d01
84e9d69
c3615a1
6a7750c
8be4d5a
dbdbdd4
adc2809
d9f1904
5f4fe46
72a183d
a6ef9cb
a12cb95
b3aafb0
c51672f
861ef58
3805792
fbedaf4
76bec99
eec789e
d80eed6
c88879e
5a29597
154a622
a6d61fa
86faf86
ac2b639
f3f6330
ce8c202
05537fd
3c302e6
937e596
ef9c5e9
6662fc7
0308f1e
ec49414
f7eda46
e3b169d
1370bbe
a5de0b7
0964c7a
0af66e1
0f6d262
bd1b6aa
0cad4ed
3b6ae69
f3940a1
c71fbd1
10fbf3d
794f1ed
21ef587
2d1dd54
19212fa
8f56b88
1e3163f
1d55e06
fb5d2a9
1f200db
c8e7408
148c2f3
c18f464
2aa07a0
2759010
7f32d73
cd82ad8
1e7b74c
4808df4
e45968b
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -608,15 +608,12 @@ def test_call_submodel_scaler_method_no_scaler(self, caplog): | |
m.b = Block([1, 2, 3]) | ||
|
||
sb = CustomScalerBase() | ||
sb.call_submodel_scaler_method(m, "b", method="dummy_method", overwrite=True) | ||
sb.call_submodel_scaler_method(m.b, method="dummy_method", overwrite=True) | ||
|
||
for bd in m.b.values(): | ||
assert not hasattr(bd, "_dummy_scaler_test") | ||
|
||
assert ( | ||
"No default Scaler set for unknown.b. Cannot call dummy_method." | ||
in caplog.text | ||
) | ||
assert "No default Scaler set for b. Cannot call dummy_method." in caplog.text | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I wonder if only referencing dummy_method in error message may confuse maintainer in the future There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do you have a suggestion for a better message? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. "No default Scaler set for b. Cannot call dummy_method created for testing purposes." There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The message is generated dynamically using the name of the method the user asked for. In this test case, the method I asked for was named "dummy_method", but a user would see the name of the method they specified. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Then I think it's fine |
||
|
||
@pytest.mark.unit | ||
def test_call_submodel_scaler_method_default_scaler(self, caplog): | ||
|
@@ -629,12 +626,12 @@ def test_call_submodel_scaler_method_default_scaler(self, caplog): | |
bd.default_scaler = DummyScaler | ||
|
||
sb = CustomScalerBase() | ||
sb.call_submodel_scaler_method(m, "b", method="dummy_method", overwrite=True) | ||
sb.call_submodel_scaler_method(m.b, method="dummy_method", overwrite=True) | ||
|
||
for bd in m.b.values(): | ||
assert bd._dummy_scaler_test | ||
|
||
assert "Using default Scaler for unknown.b." in caplog.text | ||
assert "Using default Scaler for b." in caplog.text | ||
|
||
@pytest.mark.unit | ||
def test_call_submodel_scaler_method_user_scaler(self, caplog): | ||
|
@@ -649,8 +646,7 @@ def test_call_submodel_scaler_method_user_scaler(self, caplog): | |
|
||
sb = CustomScalerBase() | ||
sb.call_submodel_scaler_method( | ||
m, | ||
"b", | ||
m.b, | ||
method="dummy_method", | ||
submodel_scalers=scaler_map, | ||
overwrite=False, | ||
|
@@ -659,7 +655,7 @@ def test_call_submodel_scaler_method_user_scaler(self, caplog): | |
for bd in m.b.values(): | ||
assert not bd._dummy_scaler_test | ||
|
||
assert "Using user-defined Scaler for unknown.b." in caplog.text | ||
assert "Using user-defined Scaler for b." in caplog.text | ||
|
||
@pytest.mark.unit | ||
def test_call_submodel_scaler_method_user_scaler_class(self, caplog): | ||
|
@@ -674,8 +670,7 @@ def test_call_submodel_scaler_method_user_scaler_class(self, caplog): | |
|
||
sb = CustomScalerBase() | ||
sb.call_submodel_scaler_method( | ||
m, | ||
"b", | ||
m.b, | ||
method="dummy_method", | ||
submodel_scalers=scaler_map, | ||
overwrite=False, | ||
|
@@ -684,27 +679,4 @@ def test_call_submodel_scaler_method_user_scaler_class(self, caplog): | |
for bd in m.b.values(): | ||
assert not bd._dummy_scaler_test | ||
|
||
assert "Using user-defined Scaler for unknown.b." in caplog.text | ||
|
||
@pytest.mark.unit | ||
def test_call_submodel_scaler_method_invalid_method(self): | ||
# Dummy up a nested model | ||
m = ConcreteModel() | ||
m.b = Block([1, 2, 3]) | ||
|
||
scaler_map = ComponentMap() | ||
scaler_map[m.b] = DummyScaler() | ||
|
||
sb = CustomScalerBase() | ||
|
||
with pytest.raises( | ||
AttributeError, | ||
match="Scaler for unknown.b does not have a method named foo.", | ||
): | ||
sb.call_submodel_scaler_method( | ||
m, | ||
"b", | ||
method="foo", | ||
submodel_scalers=scaler_map, | ||
overwrite=False, | ||
) | ||
assert "Using user-defined Scaler for b." in caplog.text |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this going to give human readable output, or is it going to be some Pyomo thing in brackets?
Consider using
submodel.name
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It will end up being
submodel.name
- it is not obvious but the f-string magic casts it to a string.