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

Custom Model Demo Rework #1390

Merged
merged 8 commits into from
May 17, 2024

Conversation

MarcusHolly
Copy link
Contributor

Summary/Motivation:

Revamps the files previously living in the custom model demo folder to function as proper tutorials inside of Jupyter Notebook

Changes proposed in this PR:

  • Adds jupyter notebook tutorials for creating and using unit models and property models (4 files in total)
  • Deletes the existing custom model demo folder

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 May 15, 2024
@MarcusHolly MarcusHolly self-assigned this May 15, 2024
@MarcusHolly MarcusHolly added the 1.0 Hard requirement for the 1.0 release label May 15, 2024
@MarcusHolly
Copy link
Contributor Author

MarcusHolly commented May 15, 2024

@lbianchi-lbl Do you know how I can resolve this notebook failure?

@lbianchi-lbl
Copy link
Contributor

image

The kernelspec name should be changed to watertap-dev (the same as the existing notebooks). If the environment was set up according to this how-to guide, you should be able to do it directly from the Jupyter(Lab) interface.

Copy link

codecov bot commented May 15, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 93.92%. Comparing base (929e539) to head (b573554).
Report is 73 commits behind head on main.

Additional details and impacted files
@@             Coverage Diff             @@
##             main    #1390       +/-   ##
===========================================
+ Coverage   41.15%   93.92%   +52.76%     
===========================================
  Files          97      335      +238     
  Lines       10572    35620    +25048     
===========================================
+ Hits         4351    33456    +29105     
+ Misses       6221     2164     -4057     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@adam-a-a
Copy link
Contributor

This is great, @MarcusHolly ! Good timing as we are aiming to spruce up tutorials and possibly use for upcoming workshops. I am thinking about whether we should bring in #1178 or this first. #1178 adds a landing page for all tutorials, and I repaired/renamed some tutorials that we have now. Maybe this one first, then I can add these new tutorials to the landing page. Thoughts @MarcusHolly @zacharybinger @ElmiraShamlou @kurbansitterley @luohezhiming ?

@MarcusHolly
Copy link
Contributor Author

This is great, @MarcusHolly ! Good timing as we are aiming to spruce up tutorials and possibly use for upcoming workshops. I am thinking about whether we should bring in #1178 or this first. #1178 adds a landing page for all tutorials, and I repaired/renamed some tutorials that we have now. Maybe this one first, then I can add these new tutorials to the landing page. Thoughts @MarcusHolly @zacharybinger @ElmiraShamlou @kurbansitterley @luohezhiming ?

@adam-a-a I agree - I think we can merge this one first and then update #1178

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.

Great! This is a priority as it may come in handy for upcoming workshop. I did not thoroughly check for typos, so if second reviewer could do that, that would be much appreciated.

@adam-a-a adam-a-a added Priority:High High Priority Issue or PR and removed Priority:Normal Normal Priority Issue or PR labels May 17, 2024
Copy link
Contributor

@savannahsakhai savannahsakhai left a comment

Choose a reason for hiding this comment

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

LGTM. I did not see any typos.

@adam-a-a adam-a-a merged commit d5f3b6b into watertap-org:main May 17, 2024
26 checks passed
@MarcusHolly MarcusHolly mentioned this pull request May 22, 2024
10 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
1.0 Hard requirement for the 1.0 release Priority:High High Priority Issue or PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants