Conversation
- in config.yml, add the option for the init_file - in the jupyter notebook, removed the hard coded dependency on the init_file, and fixed the range for plotting the time series.
|
@gunterl -- Do you mind if I push some changes to your branch just to get it to pass the automated testing done by github? |
|
@mnlevy1981 not at all, thanks for looking at this. |
Needed to pass the CI testing on github
mnlevy1981
left a comment
mnlevy1981
left a comment
There was a problem hiding this comment.
The update to add init_file is great; I have some comments about how you modified the plot limits, but the only important point is the one about what happens when base_case_name = None.
| "print(f\"avg_smb_case_climo, has shape {np.shape(avg_smb_case_climo)}\")\n", | ||
| "print(f\"avg_smb_case_timeseries has shape {np.shape(avg_smb_case_timeseries)}\")\n", | ||
| "print(f\"avg_smb_base_case_climo has shape {np.shape(avg_smb_base_case_climo)}\")\n", | ||
| "print(f\"avg_smb_base_timeseries has shape {np.shape(avg_smb_base_timeseries)}\")\n", | ||
| "print(f\"avg_smb_obs_timeseries has shape {np.shape(avg_smb_obs_timeseries)}\")\n", |
There was a problem hiding this comment.
@gunterl do you want these print statements to remain, or were they just part of some debugging while you set up numpy calls to compute ymin1, ymin2, ymax1, and ymax2?
| "ymin1 = np.array(\n", | ||
| " [avg_smb_base_case_climo, avg_smb_base_timeseries, avg_smb_obs_timeseries]\n", | ||
| ").min()\n", | ||
| "ymin2 = np.array([avg_smb_case_climo, avg_smb_case_timeseries]).min()\n", | ||
| "ymax1 = np.array(\n", | ||
| " [avg_smb_base_case_climo, avg_smb_base_timeseries, avg_smb_obs_timeseries]\n", | ||
| ").max()\n", | ||
| "ymax2 = np.array([avg_smb_case_climo, avg_smb_case_timeseries]).max()\n", | ||
| "\n", | ||
| "ymin = np.array([ymin1, ymin2]).min() - 50\n", | ||
| "ymax = np.array([ymax1, ymax2]).max() + 50\n", |
There was a problem hiding this comment.
I think you're going to run into problems with ymin1 and ymax2 if base_case_name is None; also, if you just want biggest / smallest values from three (or five) arrays, you can do something like
all_data = np.concatenate([avg_smb_case_climo, avg_smb_case_timeseries, avg_smb_obs_timeseries])
if base_case_name:
all_data = np.concatenate([all_data, avg_smb_base_case_climo, avg_smb_base_timeseries])
ymin = all_data.min() - 50.
ymax = all_data.max() + 50.| "display_name": "NPL 2025a", | ||
| "language": "python", | ||
| "name": "cupid-analysis" | ||
| "name": "npl-2025a" |
There was a problem hiding this comment.
I'll probably revert these metadata changes before committing :)
| "nbconvert_exporter": "python", | ||
| "pygments_lexer": "ipython3", | ||
| "version": "3.11.4" | ||
| "version": "3.12.8" |
There was a problem hiding this comment.
I'll fix this one, too -- it may end up getting updated, since we're now pinning 3.13.11 in (cupid-analysis)
2 participants
Description of changes:
All PRs Checklist:
pre-commitchecks passed (#8 in Adding Notebooks Guide)?New notebook PR Additional Checklist (if these do not apply, feel free to remove this section):
parameters? These can cause confusing warnings that show up asDAG build with warnings./glade/campaign/cesm/development/cross-wg/diagnostic_framework/CUPiD_obs_dataand ensured that it follows this format within that directory:COMPONENT/analysis_datasets/RESOLUTION/PROCESSED_FIELD_TYPE?