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

Raven and Teal integration #177

Merged
merged 37 commits into from
Mar 30, 2023

Conversation

radhakrishnatg
Copy link
Contributor

Addresses issue:

#86 and #90

Summary/Motivation:

This PR replaces #150

Changes proposed in this PR:

  • Adds a Jupyter notebook that demonstrates usage of TEAL for constructing cashflow expressions
  • Also add utilities to construct synthetic realizations

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.md and COPYRIGHT.md 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.

GabrielSoto-INL and others added 27 commits August 19, 2022 08:04
Copy link

@joshua-cogliati-inl joshua-cogliati-inl left a comment

Choose a reason for hiding this comment

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

Looks fine to me.

Comment on lines +18 to +19
# First, we add TEAL to the current Python path. Note that DISPATCHES, TEAL, and RAVEN are all
# assumed to be subdirectories within the same directory.
Copy link

@joshua-cogliati-inl joshua-cogliati-inl Mar 20, 2023

Choose a reason for hiding this comment

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

I think this comment is referring to code in a previous version of this file, and so can now be removed.

dguittet
dguittet previously approved these changes Mar 22, 2023
Copy link
Contributor

@dguittet dguittet left a comment

Choose a reason for hiding this comment

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

Thanks @radhakrishnatg I was able to use the install instructions and ARMA_Model files to generate synthetic LMPs

@dguittet dguittet mentioned this pull request Mar 27, 2023
@lbianchi-lbl
Copy link
Contributor

@radhakrishnatg I tried running the notebook through the test framework locally in a Windows VM in an environment where RAVEN and TEAL are installed. It seems to run just fine, but then ultimately fails because of one of the cells exceeding the default timeout (2000 s -> ~30 min). Is this in line with the runtime you get when running locally? Is there any way this example could be tweaked so that the notebook executes in 10 minutes or so?

image

@radhakrishnatg
Copy link
Contributor Author

@lbianchi-lbl I could run all the cells in the notebook within 10 minutes on my local machine. I do not know why that cell is taking more than half an hour. Also, I'm running the simplest case possible in that cell. So, could you please try running the notebook once again and let me know if it still takes longer than half an hour?

@lbianchi-lbl
Copy link
Contributor

@radhakrishnatg OK, I see. I've tried re-running and the results are the same. I guess my resource-starved VM might be to blame, in which case we could just check how long it takes on the GHA runners. In the off-chance that some package version might be affecting that, could you run pip list in your local environment and paste the output here?

@radhakrishnatg
Copy link
Contributor Author

radhakrishnatg commented Mar 28, 2023

@lbianchi-lbl I do not have my work laptop (which I used to run the notebook) with me. I will be out of office starting tomorrow and I will be back Thursday of next week. Is there any other thing I could try to know what is causing it take more time? Also, the cell which is causing the issue is also present in the multiperiod_design_pricetaker.ipynb notebook. It is strange why it can run in less than 30 min in the other notebook but not in this notebook.

@lbianchi-lbl
Copy link
Contributor

@radhakrishnatg No worries, I can look into it. Worst case, we can skip that cell since the RAVEN/TEAL setup happens in the first cells (which seem to run just fine).

Also, good point about multiperiod_design_pricetaker.ipynb - I can try to run the same exact command using that notebook and see if the long runtime also occurs.

@codecov
Copy link

codecov bot commented Mar 29, 2023

Codecov Report

Patch and project coverage have no change.

Comparison is base (b243bfa) 93.74% compared to head (cbeef5b) 93.74%.

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #177   +/-   ##
=======================================
  Coverage   93.74%   93.74%           
=======================================
  Files          59       59           
  Lines        7130     7130           
=======================================
  Hits         6684     6684           
  Misses        446      446           

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

@lbianchi-lbl
Copy link
Contributor

It looks like running the TEAL notebook in CI mostly works, but there seems to be a solver error causing cells 28 and 29 to fail.

Given that:

  • The notebook had already been approved by reviewers before the reviews were "invalidated" after merging in unrelated changes
  • We're not currently able to reproduce this behavior locally (where the notebook either works fine, as per @radhakrishnatg and the reviewers, or fails in a different way, as per my testing)

I'll go ahead and merge this after having:

@joshua-cogliati-inl
Copy link

Thank you @lbianchi-lbl

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Priority:High High Priority Issue or PR
Projects
No open projects
7 participants