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

Update Costing in Dye Desal Flowsheet #1485

Open
wants to merge 31 commits into
base: main
Choose a base branch
from

Conversation

MarcusHolly
Copy link
Contributor

@MarcusHolly MarcusHolly commented Sep 3, 2024

Fixes/Resolves:

Updates the dye desal flowsheet in preparation for a report

Changes proposed in this PR:

  • Addresses how costing is handled for certain sets of configuration options
  • Corrects minor typos in the flowsheet

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.

@MarcusHolly MarcusHolly added the Priority:Normal Normal Priority Issue or PR label Sep 3, 2024
@MarcusHolly MarcusHolly self-assigned this Sep 3, 2024
@MarcusHolly MarcusHolly changed the title Update Costing in Dye Desal Flowshee Update Costing in Dye Desal Flowsheet Sep 3, 2024
@MarcusHolly MarcusHolly marked this pull request as ready for review October 4, 2024 14:09
m.fs.brine_cost = Expression(
expr=(
-1
* m.fs.zo_costing.utilization_factor
Copy link
Contributor

Choose a reason for hiding this comment

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

Is the zero order costing utilization factor same as ro.costing? Can you tell me some background why we need to use zero order costing in this flowsheet?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, both utilization factors are set to 0.85. Zero order costing is needed to cost the ZO nanofiltration and pump models The dye_desalination_global_costing.yaml file is also linked to the zero order costing and provides values like dye_disposal_cost, recovered_water_cost, electricity_cost, etc.

@adam-a-a
Copy link
Contributor

Just as a reminder for both of us--should double-check capital recovery factor values for both ZO and "detailed" costing packages and ensure that they are equivalent and line up with what was used for analyses.

@MarcusHolly
Copy link
Contributor Author

Just as a reminder for both of us--should double-check capital recovery factor values for both ZO and "detailed" costing packages and ensure that they are equivalent and line up with what was used for analyses.

I've set the capital recovery factor for ro_costing to 0.065, which is the value used by the zo_costing and our analyses

m.fs.ro_costing.base_currency = pyunits.USD_2020
m.fs.ro_costing.utilization_factor.fix(0.85)
# Assume the same capital recovery factor as zo_costing
m.fs.ro_costing.capital_recovery_factor.fix(0.065051435)
Copy link
Contributor

Choose a reason for hiding this comment

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

@MarcusHolly any reason why we don't just point to the variable itself to get the value instead of hard-coding here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Nope - I'll update this

if value(m.fs.brine_cost) > 0:
return pyunits.convert(
m.fs.brine_cost,
to_units=pyunits.USD_2020 / pyunits.year,
Copy link
Contributor

Choose a reason for hiding this comment

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

Something to keep in mind for subsequent PR and report modifications: would be good to move away from 2020 currency since it's more of an anomaly.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@adam-a-a So what should be used instead?

Copy link
Contributor

Choose a reason for hiding this comment

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

Could go with 2023

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