-
Notifications
You must be signed in to change notification settings - Fork 14
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
Add ports / MCT Model #73
Conversation
Ok, I can now setup an Next steps should address the ports. Please add some tests so that we have something to work with. Also, we should work on adding the multiplex features (see #70). |
After some brainstorming, @hannahlanzrath and I tried to come up with some better structure for storing connections. Here are some general ideas.
For now, we went with Case 4. We'll see how this works out when having to calculate flow rates. |
…test Unit operation for MCT
…rix and update TODOs
…utre, change CSTR tests to reflect new dict structure
…ry port, Test_flow_sheet.test_connections runs without errors.
def create_MCT(self): | ||
mct = MCT(ComponentSystem(1), nchannel=3, name='test') | ||
|
||
mct.length = length |
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.
We should consider moving this outside the Test_Unit_Operation
class definition s.t. we could potentially reuse the same fixture for other tests.
(Same goes for all other units / binding / reaction models)
'exchange_matrix': exchange_matrix, | ||
'flow_direction' : 1, #TODO: Update when multiplex is implemented | ||
} | ||
np.testing.assert_equal(parameters_expected, {key: value for key, value in mct.parameters.items() if key != 'discretization'}) #Why does mct give out discretization parameters and e.g. cstr does not? |
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.
because a Cstr has no spatial discretization. In fact, its core assumption is a spatial homogeneity.
self.assertTrue(mct.nchannel*mct.component_system.n_comp == mct.c.size) | ||
|
||
|
||
|
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.
please remove the white space.
) | ||
|
||
|
||
# @n_ports.setter |
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.
Remove code that is commented out or add TODO flag.
Sorry, merged the wrong branch. 😓 |
This PR adds ports to UnitOperations in CADET process which are required by models such as the 2D-GRM.
For starters, we will test this with the Multi-Channel Transport-Model
To fix #3, we also need to fix #70.
To do
add_connection
methodFlowSheet
discretization
configurationOpen question
Should the MCT (and other potential 2D models) inherit from
TubularReactor
? How would this look like.Should every channel of the MCT have a port by default?