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

coupled model errors including rof month_annual_flow.ipynb has empty slice error #140

Closed
TeaganKing opened this issue Oct 2, 2024 · 7 comments · Fixed by #206
Closed
Assignees
Labels
bug Something isn't working hackathon lnd

Comments

@TeaganKing
Copy link
Collaborator

Describe the bug
In the rof month_annual_flow.ipynb, there is an empty slice error resulting from "inconsistent observation data length across the sites (some sites provide the data for a short period). No observed flow data is available depending analysis (or simulation) periods." (@nmizukami)

To Reproduce
Run the month_annual_flow.ipynb in the coupled models example.

Expected behavior
It'd be great to resolve this error with some if statements that could adjust the expected time period over which the calculation is done.

Additional context
Naoki was planning to adjust this in another PR.

@TeaganKing TeaganKing added bug Something isn't working lnd labels Oct 2, 2024
@TeaganKing
Copy link
Collaborator Author

Some additional coupled model errors include:

  • a register_cmap issue in ocean_surface.ipynb
  • an adf_diag import error in adf quick run

@TeaganKing TeaganKing changed the title rof month_annual_flow.ipynb has empty slice error coupled model errors including rof month_annual_flow.ipynb has empty slice error Jan 17, 2025
@mnlevy1981
Copy link
Collaborator

  • an adf_diag import error in adf quick run

I would support removing adf_quick_run.ipynb example now that we have link_to_ADF.ipynb working; I'm not terribly surprised the notebook fails, since we've updated ADF and moved stuff around recently.

@TeaganKing
Copy link
Collaborator Author

The ADF quick run notebook removal is being done in #198

@TeaganKing
Copy link
Collaborator Author

The ocean surface notebook error is also addressed in #198, so this issue ticket should really only still refer to the rof empty slice error.

@nmizukami
Copy link
Member

Hi @mnlevy1981 and @TeaganKing , I will look into this soon (maybe Friday)!

@nmizukami
Copy link
Member

I started looking at this, but I might have difficulty tracking down where this happens. I guess that this warning (or was this actually error??) happened earlier in the PR, and several revisions after this may have resolved this? I just ran the latest version of cupid with rof only, I did not get any warning. If this problem still exist, I need some help reproducing the error or warning.

When this issue was raised, I thought the problem was from numpy nanmean for all NaN elements (here).

I get this message from my old notebook.

I believe this is because current version does not use np.nanmean for observation, but just use xarray dataset mean (which skip NaN for float). this is the line (under 2.5 D19 discharge data).

There is a few places where np.nanmean is used for computing mean of the simulation. I assume that there is no NaN in simulation (Observations have missing values, depending on date and observation sites). It would be better to replace np.nanmean with just np.mean or xarray mean.

@nmizukami
Copy link
Member

nmizukami commented Mar 14, 2025

I was confused when I read through PR #126 . This problem happens now in "5.1 Compute metris at all the sites (no plots nor tables)". This used to be "3.1 Compute metris at all the sites (no plots nor tables)" in this commit with section number wrong...

This section has not changed, I can test with old data and try to change this section to remove the warning.

Note that with current example dataset, there is no effect of the changes I will make on this section.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working hackathon lnd
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants