Repository navigation
fix SolidsCentrifuge costing: use the pusher correlation and cost N centrifuges in parallel - #259
Merged
Merged
Conversation
…entrifuges in parallel SolidsCentrifuge._design selected its cost correlation with `68040*(ts**0.5) if centrifuge_type else 170100*(ts**0.3)`. The centrifuge_type setter only admits the non-empty strings 'reciprocating_pusher' and 'scroll_solid_bowl', so the condition was always true: every reciprocating pusher was priced with the scroll solid bowl correlation and the pusher correlation was dead code. The same code computed 'Number of centrifuges' = ceil(S/S_max) but never used it, so loadings above the upper limit of the correlation were extrapolated as a single machine. Both regressions date to 203a0a9 (2021), which replaced the scroll-only `@cost('Solids loading', cost=68040, CE=567, n=0.50, ub=40, N='Number of centrifuges')` with hand-written code to support two types. The decorator had costed N = ceil(S/40) parallel units at S/N each; the rewrite tested the truthiness of the type string instead of its value and dropped the N scaling. Seider et al., Table 16.32 (CE = 567; 3rd ed. Table 22.32 at CE = 500 gives 60,000 and 150,000), per centrifuge with S in short ton/hr of solids: - continuous scroll solid bowl: 68,040 S^0.50, S = 2-40 - continuous reciprocating pusher: 170,100 S^0.30, S = 1-20 The fix looks up the coefficients by type (new class attribute `purchase_cost_coefficients`, next to `solids_loading_range`) and costs N = ceil(S/S_max) centrifuges at S/N each via `parallel['Centrifuges']`, the same convention as @cost and LiquidsCentrifuge. Power stays 1.40 kWh/m3 of feed (not scaled by N), F_BM stays 2.03, loadings below the lower limit still only warn, and zero solids gives zero cost (no division by N = 0). Effect on default costs (approved by S. Bhagwat): scroll centrifuges at <= 40 ton/hr are unchanged. Pusher centrifuges (the high-rate WWT SludgeCentrifuge) rise 112-289% in purchase cost at the loadings found in HP/TAL/cornstover biorefineries (installed equipment +0.04-0.30%, selling price +0.02-0.16%). Scroll units above 40 ton/hr (succinic corn/cornstover, N = 4-7) rise 100-165% (installed equipment ~+0.35%, price +0.13-0.15%). The wastewater MESP doctest still prints $1.4/gal (1.3729 -> 1.3739); no doctest output changed. Validation: new tests/test_solids_separation.py (type-specific cost at 10 ton/hr, N = 2/3 parallel costs at 60 ton/hr, power not scaled by N, zero solids) fails on the old code for the two cost tests and passes now. Full suite (IBO_2026, --noconftest workaround): 87 failed, 463 passed, 62 skipped, with the identical failure list and reasons as the unmodified baseline (87 failed, 459 passed, 62 skipped); +4 passed are the new tests. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
yoelcortes
approved these changes
Oct 9, 2026
yoelcortes
left a comment
Member
There was a problem hiding this comment.
Looks good! Thank you for catching this, taking the time to fix it, and adding tests!
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Issue addressed
SolidsCentrifuge._designpicked its cost correlation withThe
centrifuge_typesetter only accepts the non-empty strings'reciprocating_pusher'and'scroll_solid_bowl', so the condition is always true. As a result:'Number of centrifuges' = ceil(S/S_max)was computed but never applied, so loadings above the correlation's upper limit were costed as one extrapolated machine.Underlying cause
Both regressions date to 203a0a9 (2021). That commit replaced the scroll-only
@cost('Solids loading', cost=68040, CE=567, n=0.50, ub=40, N='Number of centrifuges'), which costed N = ceil(S/40) parallel units at S/N each, with hand-written code to support two types. The rewrite tested the type string's truthiness instead of its value, and dropped the N scaling.Published correlations: Seider et al., Product and Process Design Principles, Table 16.32 (CE = 567). The 3rd edition's Table 22.32 has the same values at CE = 500 (60,000 and 150,000). Costs are per centrifuge, with S in short ton/hr of solids:
Fix
purchase_cost_coefficients, sits next tosolids_loading_range, and_designlooks the coefficients up by type.parallel['Centrifuges'] = N. This is the same convention as@costandLiquidsCentrifuge.F_BMstays at 2.03.'Centrifuges'.Effect on default costs
I measured this by simulating 14 Bioindustrial-Park configurations before and after the fix.
SludgeCentrifuge(cornstover, HP ×4, TAL ×4, oxalic)The
biosteam.wastewaterMESP doctest still prints$1.4/gal(1.3729 → 1.3739). No doctest output changed.Validation
New regression tests in
tests/test_solids_separation.py:The two cost tests fail on the old code (pusher priced at 215,351 instead of 339,694; 60 ton/hr scroll priced at 527,500 instead of 745,998) and pass with the fix.
Full suite (local stack;
--noconftest, because the repoconftest.pyuses thepathhook argument that pytest 9 removed). Some failures are pre-existing on unmodified master because of local stack drift:33ae06abThe failure list and failure reasons are identical; the 4 extra passes are the new tests.
Independent code review: it confirmed the
parallelhandling, the_setup/_load_costsinteraction (no double counting on repeated simulation), and theSludgeCentrifugepaths (including SKIPPED). It found that nothing downstream reads the'Centrifuges'cost item directly.The branch merges cleanly onto current
master(5b2836c). The upstream commits since the base don't touch costing.Possible follow-ups (not in this PR)
@costitem that hasub.reciprocating_pusherand runs below that correlation's 1 ton/hr minimum. Most of the pusher cost increase comes from extrapolating there. Whether to clamp low loadings, or to usescroll_solid_bowlfor sludge, is a separate decision about default costs.🤖 Generated with Claude Code