Skip to content
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

OARO remove spiral-wound ModuleType #1461

Merged
merged 13 commits into from
Jul 17, 2024

Conversation

zacharybinger
Copy link
Contributor

Fixes/Resolves:

#1380

Summary/Motivation:

Implementation of OARO with a spiral-wound module is more complex than the initial implementation represented. Enabling spiral-wound operation of OARO would likely require another PR with more creative/extensive changes that I don't think are possible with our current capabilities. More precisely spiral-wound OARO would require a 2D model.

Changes proposed in this PR:

  • Removing ModuleType.spiral_wound support for OARO models

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.

Copy link
Contributor

@MarcusHolly MarcusHolly left a comment

Choose a reason for hiding this comment

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

LGTM

@adam-a-a
Copy link
Contributor

adam-a-a commented Jul 3, 2024

Enabling spiral-wound operation of OARO would likely require another PR with more creative/extensive changes that I don't think are possible with our current capabilities

Just want to clarify that the underlying capabilities are there (e.g., IDAES has at least one 2d model that I can recall off the top of my head)--it is just that we haven't implemented any 2d models in WaterTAP thus far. Although such an effort would require a lot of time and effort, I just want to make sure that anyone reading through this PR understands that such a model is possible in WaterTAP and would be facilitated by some of the building blocks in IDAES.

Copy link
Contributor

@adam-a-a adam-a-a left a comment

Choose a reason for hiding this comment

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

LGTM- the only necessary changes are updating the documentation for OARO 0D and 1D, wherever spiral-wound modules are mentioned.

@ElmiraShamlou
Copy link
Contributor

@zacharybinger @adam-a-a How about keeping the spiral wound option for the OARO 0D model instead of fully removing it? For velocity calculations in the permeate channel, we can redefine the cross-sectional area as follows:

Velocity (permeate channel) = Qavg / (Porosity * (Module Length / 2) * Channel Height)
where:
Module Length: Length of the module (used instead of width to reflect the perpendicular flow direction)
Division by 2: Accounts for the effects of baffles in the permeate channel

@adam-a-a
Copy link
Contributor

adam-a-a commented Jul 3, 2024

@zacharybinger @adam-a-a How about keeping the spiral wound option for the OARO 0D model instead of fully removing it? For velocity calculations in the permeate channel, we can redefine the cross-sectional area as follows:

Velocity (permeate channel) = Qavg / (Porosity * (Module Length / 2) * Channel Height) where: Module Length: Length of the module (used instead of width to reflect the perpendicular flow direction) Division by 2: Accounts for the effects of baffles in the permeate channel

Hmm.. maybe that could work. Does anyone know if we have any peer-reviewed literature on spiral wound OARO performance to test against?

@zacharybinger
Copy link
Contributor Author

.. maybe that could work. Does anyone know if we have any peer-reviewed literature on spiral wound OARO performance to test against?

Not to my knowledge, nor in the quick literature search I just did

@ksbeattie ksbeattie added the Priority:Normal Normal Priority Issue or PR label Jul 11, 2024
@adam-a-a
Copy link
Contributor

adam-a-a commented Jul 12, 2024

@zacharybinger - just a reminder that this PR should be good to go soon--just waiting on the corresponding updates to the documentation to reflect removal of SW module type from OARO. We can revisit @ElmiraShamlou 's suggestion via a subsequent issue/PR as needed.

@zacharybinger
Copy link
Contributor Author

@adam-a-a Ok, once these checks pass it should be good to go then. I removed any mention of spiral-wound from the docs, there were only a few lines

Copy link
Contributor

@adam-a-a adam-a-a left a comment

Choose a reason for hiding this comment

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

LGTM

@adam-a-a adam-a-a merged commit 4ed2395 into watertap-org:main Jul 17, 2024
21 checks passed
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.

5 participants