Closed AOstenfeld closed 4 years ago
Nice, this is looking really good! I have two recommendations.
First, rather than have a csv
diagnostic like would be used for the "command line" version of the app, this notebook should use some sort of other custom diagnostic. I'm thinking something that gathers the output in a buffer (could even use the CSVOutputUtility
for this) but instead of writing to file at the end, it plots the data. That way you can see the code output right there in the notebook.
Second, I think a tutorial should have more text/equations around the code, to really describe in detail what you are trying to achieve with the code. If you can add more, that would be really helpful for someone who is trying to use this tutorial to learn about turboPy.
I was trying to address the first point, and I'm not sure how to do that outside of editing the code in core.py. Should we add functionality to turboPy that would allow it to plot the outputs directly?
Edit: This would probably involve either creating a new class similar to CSVOutputUtility, but would plot the data instead of saving it, or just inserting some code somewhere into the run() method that would plot the outputs.
I think it's ok to add matplotlib to environment.yml and then use it in your example notebook. No need to add plotting to turbopy itself, just your notebook. Does this make sense?
matplotlib should go in the environment.yml for this repo, not the core turboPy repo
How is this looking?
Looks really nice! There are some minor things that I'll probably change, but most are just editorial in nature.
Can you add a sentence or two at the top explaining what the equations are that need to be solved for this problem?
Once you have that, we can merge this file into the repo, and I can make the minor edits myself.
It's a start.
Addresses #5