Skip to content

MCAS in SWRO Flowsheet - #1849

Open
adam-a-a wants to merge 26 commits into
watertap-org:mainfrom
adam-a-a:swro_mcas
Open

MCAS in SWRO Flowsheet#1849
adam-a-a wants to merge 26 commits into
watertap-org:mainfrom
adam-a-a:swro_mcas

Conversation

@adam-a-a

Copy link
Copy Markdown
Contributor

Fixes/Resolves:

(replace this with the issue # fixed or resolved, if no issue exists then a brief statement of what this PR does)

Summary/Motivation:

Changes proposed in this PR:

Legal Acknowledgement

By contributing to this software project, I agree to the following terms and conditions for my contribution:

  1. I agree my contributions are submitted under the license terms described in the LICENSE.txt file at the top level of this directory.
  2. I represent I am authorized to make the contributions and grant the license. If my employer has rights to intellectual property that includes these contributions, I represent that I have received permission to make contributions and grant the required license on behalf of that employer.

@adam-a-a

Copy link
Copy Markdown
Contributor Author

@sufikaur @ksbeattie this is the other PR that could potentially make it into the patch release

@ksbeattie ksbeattie added the Priority:Normal Normal Priority Issue or PR label Jul 23, 2026
@adam-a-a
adam-a-a marked this pull request as ready for review July 29, 2026 03:20
@adam-a-a

Copy link
Copy Markdown
Contributor Author

@sufikaur This followup PR that I was hoping to get into the patch release is actually ready! I was able to resolve the issues. There may be a couple of trivial test failures to resolve on my end (checking now). My hope is that we can still squeeze this in once we get two reviews.

@adam-a-a
adam-a-a requested a review from tristantc July 29, 2026 03:22
# RO unit
desal.RO.A_comp.fix(4.2e-12) # membrane water permeability coefficient [m/s-Pa]
desal.RO.B_comp.fix(3.5e-8) # membrane salt permeability coefficient [m/s]
desal.RO.B_comp[0, "tss"].fix(1e-10) # membrane salt permeability coefficient [m/s]

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice, so we can do this even without having to provide charge.

@@ -353,17 +318,50 @@
temperature = 298 * pyunits.K

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a minor comment. but we should probably put this into to fix_operation_function. Not critical.

m.fs.feed.properties[0].conc_mass_phase_comp["Liq", "tss"],
value(10 / conc_mass_tss),
)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I almost think, if you make this into fix_feed_conditions function, it will make a clear example of how to set feed state using concentrations and flow volume.

},
hold_state=True,
)
m.fs.properties.set_default_scaling(

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would also put all scaling stpes into a separate function.

this we can call build, fix, scale, initalize etc.

# initialize pretreatment
propagate_state(m.fs.s_feed)
flags = fix_state_vars(prtrt.intake.properties)
solve(prtrt, checkpoint="solve flowsheet after initializing pre-treatment")

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not sure why we are using prtrt/psttrt they are realy confusing and do not make it easier to read the code. I suggest we just use explicity m.fs.pretreatment etc.

@@ -601,8 +601,8 @@
m.fs.costing.base_currency = pyunits.USD_2023

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will not wor kcorrectly. The costing is already built and using default costing units loaded at build_global_params call, which happens at costing build. That means the vars are using different base_currency here then you would expect. - e.g. any vars build at global will use base currency (I think USD_2019) and any vars down stream will use 2023. This is an issue, as it means you might be down converting some of the costs rather then up based on CPI index.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ideally, we should define base cost at our desired year, and any unit costs/capex/etc have cost defined at specific year (USD_2020 or something - in many cases they just use base_currency which is what ever is the base_currecny var in the costing package. I am not sure how that impacts all equations but its inconsistent, for example your electricity price will use USD_2018 (set here, which is called here and build_global_params is called at build by base flowshetCostingblock here which means all vars called after here use USD_2018, and only anything built after build_global_parms will end up using USD_2023. which means base all aggreaget costs etc on USD_2018, build units with csoitng of USD_2023 unless they specify otherwise, and cost eelctricty on USD_2018!

yay....

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unless I can't read code... and then I am wrong and should pursue my wifes dream of running a Cat farm somewhere...

@avdudchenko avdudchenko left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall nice. but be aware of bugged up base_currency usage here. (just make sure the tests pass. Most of my comments are related to clearer organization of some code but its very optional)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Priority:Normal Normal Priority Issue or PR

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants