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

Kawan's comments #4

Open
falkamelung opened this issue Jul 24, 2024 · 1 comment
Open

Kawan's comments #4

falkamelung opened this issue Jul 24, 2024 · 1 comment

Comments

@falkamelung
Copy link
Member

I’m unfortunately still having some issues with plot_precip.. The errors that I am having now aren’t raising an error but instead the process is being automatically killed due to out of memory issues, this happened quite a few times with the Cotopaxi and I think other volcanoes. I’m not sure if this is unique to jetstream2. There is also still the issue with the name/latlon switch for some map plots.

@giacomo if you end up having time next week to start the refactor, I would suggest that you open a new branch, called refactor and start working on the changes there.

A couple things that we should prioritize for the refactor include:
1. Break up prompt subplots and simplify. (see the last commit for my suggestions)
2. A plot function should take the axes as an input parameter. This would allow greater control of the plot and means we can make one big figure of n subplots and pass each axes to a plotting function. Currently plt... is being used. This is a bigger change and you will need to get familiar with matplotlib to do so. Long term, this change will be really worth it.
https://matplotlib.org/stable/users/explain/axes/index.html
https://stackoverflow.com/questions/43482191/matplotlib-axes-plot-vs-pyplot-plot
3. Less inps arguments (start with positional/volcanoname and all the download options) - at the moment I need to comment out the 'inps.volcano_name == inps.positional' line if I want to use run_all..
4. Read through minty cli sccripts and plot functions for good templates.

Initial comments:

//precipvm/home/exouser/code/rsmas_insar/tools/Precip[2021] grep -r . -e 'KA:'
./src/precip/plotter_functions.py:# KA: Don't use import *
./src/precip/plotter_functions.py:# KA: This function should just handle plotting.
./src/precip/plotter_functions.py:# KA: Ideally it takes data and a single axes as the input and then sends this data to another function depending on style
./src/precip/plotter_functions.py: # KA: this is a useful function but should be moved to the command line script
./src/precip/plotter_functions.py: # KA: this is a useful function but should be moved to the command line script
./src/precip/plotter_functions.py: # KA: There is no else statement here and isn't there always a style if you want to plot something?
./src/precip/plotter_functions.py: # KA: lets keep saving outside of plotting functions for now
./src/precip/plotter_functions.py: # KA: no need to set an extra bool
./src/precip/plotter_functions.py: # KA: instead -> if inps.style in ['daily', 'bar', 'strength':
./src/precip/plotter_functions.py: # KA: This dataframe should be the input parameter to a plotting function so that it doesn't have to be read everytime
./src/precip/plotter_functions.py: # KA: This dataframe should be the input parameter to a plotting function so that it doesn't have to be read everytime
./src/precip/plotter_functions.py: # KA: This dataframe should be the input parameter to a plotting function so that it doesn't have to be read everytime

@falkamelung
Copy link
Member Author

falkamelung commented Jul 24, 2024

For each volcano/run it should generate one or more files that people can download if they want to.

When everything is nicely running we need to support additional datasets:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant