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

Updating NGCC Flowsheet #98

Merged
merged 38 commits into from
Aug 1, 2024
Merged

Updating NGCC Flowsheet #98

merged 38 commits into from
Aug 1, 2024

Conversation

JavalVyas2000
Copy link
Contributor

@JavalVyas2000 JavalVyas2000 commented Mar 29, 2024

Updating NGCC Flowsheet

This PR updates and moves the NGCC flowsheet from active/pow_gen to notebooks/doc/flowsheets/pow_gen.


Legal Acknowledgement

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

I agree my contributions are submitted under the license terms described in the LICENSE.txt file at the top level of this directory.
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.

📚 Documentation preview 📚: https://idaes-examples--98.org.readthedocs.build/en/98/

@lbianchi-lbl lbianchi-lbl added the Priority:Normal Normal Priority Issue or PR label Apr 4, 2024
@ksbeattie
Copy link
Member

@JavalVyas2000 news?

@@ -0,0 +1,116 @@
WARNING: Params with units must be mutable. Converting Param
Copy link
Contributor

Choose a reason for hiding this comment

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

What is this results.txt file that has been added to the HDA and methanol flowsheets? Is this relevant to the NGCC 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.

No. This was something which I was testing out with. I removed it from this PR.

@@ -107,7 +114,8 @@
},
{
"cell_type": "code",
"execution_count": 4,
"execution_count": null,
Copy link
Contributor

Choose a reason for hiding this comment

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

I noticed that none of the cells have been executed in this notebook. All the other example notebooks I've seen have their cells executed, but I'm not sure if this ultimately matters or not.

Copy link
Contributor

Choose a reason for hiding this comment

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

@andrewlee94 Should the cells be executed before this is approved?

Copy link
Member

@andrewlee94 andrewlee94 May 31, 2024

Choose a reason for hiding this comment

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

That is more a question for @lbianchi-lbl. I do not know exactly how things get run and generated.

I suspect that @JavalVyas2000 needs to run the idaesx commands to generate the full set of docs which would fill these in for him.

@ksbeattie
Copy link
Member

@JavalVyas2000 any news on this?

"metadata": {},
"source": [
"# NGCC Baseline and Turndown\n",
"Maintainer: John Eslick \n",
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we think about changing the maintainer?

"# NGCC Baseline and Turndown\n",
"Maintainer: John Eslick \n",
"Author: John Eslick \n",
"Updated, 2023-06-01 \n",
Copy link
Contributor

Choose a reason for hiding this comment

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

Please update the date here.

Copy link
Contributor

@AlexNoring AlexNoring left a comment

Choose a reason for hiding this comment

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

lgtm

Copy link
Contributor

@agarciadiego agarciadiego left a comment

Choose a reason for hiding this comment

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

LGTM

@ksbeattie ksbeattie merged commit d6e14b4 into IDAES:main Aug 1, 2024
8 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.

6 participants