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

When calculating max precipitation values, use first index too #65

Merged
merged 1 commit into from
Nov 19, 2023

Conversation

cwitting
Copy link
Contributor

Noticed that if the first index of rain values where larger than the rest, the bound calculation would be off,
as the loop below starts from 1.
I am not sure I understand why the first index does not need to be used for the temperature calculation though.

@lmarzen
Copy link
Owner

lmarzen commented Nov 19, 2023

Good catch.

Can you clarify what you mean that the first index is not used for the temperature calculation?

@lmarzen lmarzen merged commit 945d803 into lmarzen:main Nov 19, 2023
3 checks passed
@cwitting cwitting deleted the fix_index_start branch November 20, 2023 14:47
@cwitting
Copy link
Contributor Author

I mean that i do not fully understand why the loop needs to start at 1.
In the second loop below the temperature are accessed using "i - 1" as index, where as rain/pop is accessed using just "i".

@lmarzen
Copy link
Owner

lmarzen commented Nov 20, 2023

The first loop identifies the range for max/min temperature and max rainfall (lower bound for rain fall is always 0). The first loop starts at 1 since on iteration 0 for temperature, we need to initialize both the max and min temperature to the first hourly for temperature. For rain we wouldn't need to do this since we could use max(0,hourly.rain[i]).

For the second loop, the reason temperature accesses i-1 is because it must draw a line from the previous hour to the current hour. However, this is not necessary for rain/pop since it just draws a bar for each hour and is independent of the pop of any other hour.

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.

2 participants