search

IDAES / examples-pse

(ARCHIVED - use IDAES/examples) Example Python code, Jupyter Notebooks, and other files for the IDAES PSE
https://idaes.github.io/examples-pse/
Other
20 stars 36 forks source link

Adding StateJunction & Valve Unit Models into Jupyter Notebook #166

Closed MarcusHolly closed 1 year ago

MarcusHolly commented 1 year ago

Proposed changes:

Will add information for the following unit models into jupyter notebook

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 commented 1 year ago

The notebooks look good, but should have a bit more detail describing each step of the examples. I also had a few other comments on the StateJunction example.

@bpaul4 Could you specify which steps need more detail? It seems like this is a recurring comment across PRs, so I want to make sure it is adequately addressed. Also, some other examples have very few/no explanations (turbine example). Although the turbine is a much simpler model, I just wasn't sure how much detail should be attached to each step.

bpaul4 commented 1 year ago

The notebooks look good, but should have a bit more detail describing each step of the examples. I also had a few other comments on the StateJunction example.

@bpaul4 Could you specify which steps need more detail? It seems like this is a recurring comment across PRs, so I want to make sure it is adequately addressed. Also, some other examples have very few/no explanations (turbine example). Although the turbine is a much simpler model, I just wasn't sure how much detail should be attached to each step.

I appreciate the question. The initialization and solve sections are light on text, and a short summary at the end of the example would be useful to loop back to the motivation for the notebook. The examples are sufficient without the text, but it provides additional guidance and clarification for users and is worth adding.

agarciadiego commented 1 year ago

The notebooks look good, but should have a bit more detail describing each step of the examples. I also had a few other comments on the StateJunction example.

@bpaul4 Could you specify which steps need more detail? It seems like this is a recurring comment across PRs, so I want to make sure it is adequately addressed. Also, some other examples have very few/no explanations (turbine example). Although the turbine is a much simpler model, I just wasn't sure how much detail should be attached to each step.

I appreciate the question. The initialization and solve sections are light on text, and a short summary at the end of the example would be useful to loop back to the motivation for the notebook. The examples are sufficient without the text, but it provides additional guidance and clarification for users and is worth adding.

@MarcusHolly I think you could add a small explanation on why we run the initialize function (i.e., giving all values an initial value) I know it is difficult to understand for first-time users why we run initialize() and then solve(). Additionally, you use WARNING in initialize, you could add a small description to why.

MarcusHolly commented 1 year ago

Also, is it worth updating the older examples with better descriptions etc. to bring them up to the standard set by these files? Obviously this is not an immediate issue, but maybe something we (or I) should consider doing at some point down the line.

andrewlee94 commented 1 year ago

@MarcusHolly Short answer is yes, it would be nice to bring the other examples up to the same standard if we can find the resources.