MSContactor Scaler#1681
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## scaling_toolbox #1681 +/- ##
===================================================
+ Coverage 77.21% 77.30% +0.08%
===================================================
Files 395 395
Lines 63859 64106 +247
Branches 10606 10707 +101
===================================================
+ Hits 49306 49554 +248
+ Misses 12065 12064 -1
Partials 2488 2488 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
MarcusHolly
left a comment
MarcusHolly
left a comment
There was a problem hiding this comment.
I agree with former comments that it'd be nice to flesh out the documentation we have on the current scaling tools, but I recognize that 's not as high priority as getting these things merged before the end of the month
| # equality constraint involving geometry in the parent | ||
| # unit model. | ||
| "volume": DefaultScalingRecommendation.userInputRequired, | ||
| "volume_frac_stream": 2, |
There was a problem hiding this comment.
Why is this 2 as opposed to 1 or 10? Just because the average volume fraction is 0.5?
There was a problem hiding this comment.
Yeah. There will typically be two streams, and, without additional user information, we'd expect the volume to be evenly divided between streams.
| # "stream_name_phase_fraction" Vars. Because we do not know the stream | ||
| # names a priori, this value is copied for those streams as part of | ||
| # the variable_scaling_routine (if the user hasn't already set a value) | ||
| "phase_fraction": 10, |
There was a problem hiding this comment.
Same comment as above... why not 1 or 2?
There was a problem hiding this comment.
I started using 10 for phase fraction because it's what I was using for mole fraction. 2 might have been a better choice, but I put 10 elsewhere so I put it here to be consistent.
Ryan-Hughes-8
left a comment
Ryan-Hughes-8
left a comment
There was a problem hiding this comment.
Looks good. I second Marcus's comment about improving the documents and/or examples once these scalers are all merged.
* rescue mscontactor scaler from branch * test geometry scaling * dynamics scaling * Energy balance testing * test inherent reaction scaling * final stage of tests * run black * pylint
3 participants
Summary/Motivation:
Adds scaler object for MSContactor unit model.
Legal Acknowledgement
By contributing to this software project, I agree to the following terms and conditions for my contribution: