Conversation
ischoegl
force-pushed
the
sourcegen-zeroD
branch
from
0beb045 to
e8e6bd9
Compare
Introduce convenience methods similar to those introduced in ConnectorFactory. Implement transitional behavior for updated newReactor and add new newReservoir function. Further, introduce virtual FlowDevice methods to avoid casting.
ischoegl
force-pushed
the
sourcegen-zeroD
branch
from
e8e6bd9 to
7576bf6
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1878 +/- ##
==========================================
- Coverage 74.33% 74.08% -0.25%
==========================================
Files 443 443
Lines 55403 55488 +85
Branches 9108 9118 +10
==========================================
- Hits 41183 41108 -75
- Misses 11122 11285 +163
+ Partials 3098 3095 -3 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
ischoegl
force-pushed
the
sourcegen-zeroD
branch
from
7576bf6 to
40fed9e
Compare
|
@speth ... this is the last portion missing before the experimental CLib reaches parity with the traditional CLib API. Let me know if this needs further work. |
| def device_coefficient(self): | ||
| """ | ||
| Device coefficient (defined by derived class). | ||
|
|
||
| Example: | ||
|
|
||
| >>> v = Valve(res1, reactor1) | ||
| >>> v.device_coefficient = 1e-4 # Set the 'valve coefficient' | ||
|
|
||
| .. versionadded:: 3.2 | ||
| """ |
There was a problem hiding this comment.
I understand the rationale for creating a generic interface for setting this coefficient in the CLib case, but I'm concerned that in the Python case, this is just creating a redundant interface that parallels the existing PressureController.pressure_coeff and MassFlowContoller.mass_flow_coeff properties.
There was a problem hiding this comment.
I implemented this as I was hoping to have mostly parallel implementations on all APIs. At the same time, I wasn't sure that I wanted to deprecate (in both C++ and Python). If you want me to make a decision, I'd be in favor of device_coefficient as it is a more efficient implementation.
There was a problem hiding this comment.
I'm fine with having both options in place for now, with the idea that we might eventually deprecate the device-specific implementation in favor of the generic approach.
|
@speth ... thanks for the review! I addressed all suggestions, with the exception of the last one, where I am conflicted on whether to deprecate the earlier coefficient getters/setters. |
2 participants
Changes proposed in this pull request
With this PR,
clib_experimentalreaches parity with the traditionalclib.zeroDobjects to experimental CLibReactorNetwith list ofReactorBase; deprecate incremental path viaaddReactornewReservoir) and clarify future use ofnewReactor.samples... previously overlooked due to C++ samples not checked for use of deprecated methods #1603If applicable, fill in the issue number this pull request is fixing
Addresses Cantera/enhancements#220
If applicable, provide an example illustrating new features this pull request is introducing
Checklist
scons build&scons test) and unit tests address code coverage