Skip to content

Update icing-base-top.lua#32

Merged
tackandr merged 1 commit intomasterfrom
STU-29733
Apr 14, 2026
Merged

Update icing-base-top.lua#32
tackandr merged 1 commit intomasterfrom
STU-29733

Conversation

@tackandr
Copy link
Copy Markdown
Contributor

Fix issue with >=4 / <=4 comparison by using >3 / <5 since values are integers.
https://jira.fmi.fi/browse/STU-29733

Fix issue with >=4 / <=4 comparison by using >3 / <5 since values are integers.
Copy link
Copy Markdown
Contributor

@Iikkater Iikkater left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good!

@TyttiHMustonen
Copy link
Copy Markdown

The documentation of the VerticalHeightGreaterThanGrid and VerticalHeightLessThanGrid is unclear about the last argument (Nth). The explanation is the same for Nth = 0 or Nth = 1. https://github.com/fmidev/himan/blob/master/doc/plugin-hitool.md

If the Nth = 1 means that we try to find level where the parameter is bigger than threshold, and
if the Nth = 0 means that we try to find level there the parameter is smaller than threshold, then
we want it always to be 1. This because.. how i understand this, is that we want to get the first (lowest) level where icing is >= 4. And also the last (highest) level where icing is >= 4. So in both we have icing >= 4.

  1. local basedata = hitool:VerticalHeightGreaterThanGrid(IceParam, zerodata, pFL300data, thresholddata, 1)

    • With this we find the first (lowest) height where icing >= 4, and the search is done between zerodata - pFL300data.
  2. local topdata = hitool:VerticalHeightLessThanGrid(IceParam, basedata, pFL300data, thresholddata, 1)

    • With this we find the last (highest) height where icing >= 4 , and the search is done between the lowest level already found (basedata) - pFL300data.

    Okey, so the plugin doc talks about >=, not >, so 4 should already work?
    Why it doesn't?
    Maybe documentations is false, and it is > , not =>.
    If we need to include 4, we need to say > 3 in both cases?

    Maybe the functions just need to be tested what they do, or ask someone who knows more about them?
    Or maybe i didn't understand what they wanted in the ticket STU-29733
    :D

@tackandr
Copy link
Copy Markdown
Contributor Author

tackandr commented Apr 8, 2026

Okay, that's an error in the documentation. It was just copypasted I assume. 0 means last level and 1 means first.

The problem is that our method only implements > operator but not >= operator. And since the parameter is an index type containing natural numbers we don't catch the 4 since that is >= 4 but not > 4. So in this case we use > 3 to get >= 4.
The documentation gives an example of total cloudiness >= 80% but that is also in reality > 80% but not such an issue since it is a floating point number and that brings the cutoff close enough to 80% anyway.

Documentation should be fixed for this.

@TyttiHMustonen
Copy link
Copy Markdown

Okay, now i understand that the functions work like this:

  • local basedata = hitool:VerticalHeightGreaterThanGrid(IceParam, zerodata, pFL300data, thresholddata=3, 1)

This finds the first height where icing parameter is > 3, limits search between levels [zerodata, pFL300data]

  • local topdata = hitool:VerticalHeightLessThanGrid(IceParam, basedata, pFL300data, thresholddata=5, 1)

This finds the first height where icing parameter is < 5 , limits search between levels [basedata, pFL300data]

But if we want to accept icing parameter 4, 5, 6, 7, 8 and 9 should we change this ->

local topdata = hitool:VerticalHeightGreaterThanGrid(IceParam, basedata, pFL300data, thresholddata=3, 0)

So it would fine the last height where icing parameter is > 3?

@tackandr
Copy link
Copy Markdown
Contributor Author

In principle yes. There could be multiple layers of icing and we wanted to catch the top of the layer starting from base. If we have a single icing layer then the two ways should provide the same result.

@tackandr tackandr merged commit e474dc4 into master Apr 14, 2026
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