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

Feature/time series #160

Merged
merged 6 commits into from
Sep 10, 2024
Merged

Feature/time series #160

merged 6 commits into from
Sep 10, 2024

Conversation

FariborzDaneshvar-NOAA
Copy link
Collaborator

Added optional timeslot input (default=1) to compute_surrogate_percentiles() and compute_surrogate_probability_field() functions to avoid MemoryError when calculating poly1 = poly(*Z)

@codecov-commenter
Copy link

codecov-commenter commented Sep 5, 2024

Codecov Report

Attention: Patch coverage is 0% with 12 lines in your changes missing coverage. Please review.

Project coverage is 25.50%. Comparing base (f14850d) to head (9f00461).

Files with missing lines Patch % Lines
...rturbation/uncertainty_quantification/surrogate.py 0.00% 12 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #160      +/-   ##
==========================================
- Coverage   25.56%   25.50%   -0.07%     
==========================================
  Files          29       29              
  Lines        4017     4027      +10     
==========================================
  Hits         1027     1027              
- Misses       2990     3000      +10     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@SorooshMani-NOAA
Copy link
Collaborator

@FariborzDaneshvar-NOAA just for the sake of future reference, can you please include some of the multi step plots, etc. that was generated using this approach? I mean can you please attach snapshots as a PR comment?

@FariborzDaneshvar-NOAA
Copy link
Collaborator Author

@FariborzDaneshvar-NOAA just for the sake of future reference, can you please include some of the multi step plots, etc. that was generated using this approach? I mean can you please attach snapshots as a PR comment?

This update doesn't change plots. It only passes surrogate_model in chunks to Poly1(*Z) in surrogate.py.

This is what we had before for percentiles:

        surrogate_percentiles = compute_surrogate_percentiles(
            poly=surrogate_model,
            q=percentiles,
            dist=distribution,
            convert_from_log_scale=convert_from_log_scale,
        )

But with this new update, it will use multiple chunks of surrogate_model (default number of timeslots is 1):

    # use chunks of surrogate model (for each timeslot) to calculate percentiles
    surr_chunk_length = int(surrogate_model.shape[0] / timeslots)

    for timestep in range(timeslots):
        surrogate_percentiles_chunk = compute_surrogate_percentiles(
            poly=surrogate_model[
                (timestep * surr_chunk_length) : ((timestep + 1) * surr_chunk_length)
            ],
            q=percentiles,
            dist=distribution,
            convert_from_log_scale=convert_from_log_scale,
        )
        if timestep == 0:
            surrogate_percentiles = surrogate_percentiles_chunk
        else:
            surrogate_percentiles = numpy.concatenate(
                (surrogate_percentiles, surrogate_percentiles_chunk), axis=1
            )

@SorooshMani-NOAA
Copy link
Collaborator

Thanks @FariborzDaneshvar-NOAA can you verify that we get the same plots as before if you run this code on the maxelev data?

@FariborzDaneshvar-NOAA
Copy link
Collaborator Author

Here are the percentiles and probability plots of a given surrogate_model, from the old code (main branch) and the new code (feature/time_series branch) with timeslot variable:

main branch feature/time_series branch (default timeslot=1) feature/time_series branch (user-defined timeslot=1)
001_old_code_percentiles_70 002_new_code_default_percentiles_70 002_new_code_user-defined_percentiles_70
000_old_code_probability_exceeding_4_ft 001_new_code_default_probability_exceeding_4_ft probability_exceeding_4_ft

Three plots are exactly the same, confirming that the newly added variable (timeslot) doesn't change the outputs/plots.

@SorooshMani-NOAA
Copy link
Collaborator

Thank you, I'll go ahead with the merge then!

@SorooshMani-NOAA SorooshMani-NOAA merged commit 1e98e44 into main Sep 10, 2024
10 checks passed
@SorooshMani-NOAA SorooshMani-NOAA deleted the feature/time_series branch September 10, 2024 11:19
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

Successfully merging this pull request may close these issues.

3 participants