-
Notifications
You must be signed in to change notification settings - Fork 160
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
Tutorial for FSI VaR MonteCarlo #1874
Tutorial for FSI VaR MonteCarlo #1874
Conversation
I don't see that I can add a label. I assume that is on purpose. Thanks! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am going to piggy pack with the discussion that you are having with @cboneti. I think there are several more structural concerns that I have that would be best to have discussion about. Consider waiting to address my comments here until we have had that discussion.
/gcbrun |
community/modules/files/fsi-montecarlo-on-batch/FSI_MonteCarlo.ipynb
Outdated
Show resolved
Hide resolved
/gcbrun |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am not finished with my review but wanted to give some comments while I break for a bit.
/gcbrun |
@jrossthomson I went ahead and addressed the majority of my findings. Please review 1533b71 and make sure you agree with my changes. Generally I tracked these in comments if you want more context. I also needed to incorporate updates from develop to get tests to pass. I did not want to force push over your history so I manually merged in the latest copy of develop. There are still a few open items for you to address. Feel free to reach out to discuss. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think I have all the requested changes done.
8f1a75a
into
GoogleCloudPlatform:develop
Adds
docs/tutorials/fsi-montecarlo-on-batch/README.md
: Demo used for SC23.Several modules are added to support this demo. While we hope these modules are able to be reused and repurposed, they are marked as experimental and their main purpose is to support this demo.
New modules: