pds-data-dictionaries / PDS4-LDD-Issue-Repo

Issue repository for tracking all PDS4 Discipline Dictionary-related issues, new feature requests, and releases.
Apache License 2.0
2 stars 1 forks source link

[ldd-disp] Frame_rate lower limit seems oddly arbitrary #205

Open acraugh opened 2 years ago

acraugh commented 2 years ago

Issue Type Bug

Describe the issue identified (if applicable) frame_rate has a lower limit of 1 frame/s even though the attribute is real-valued. The rate seems arbitrary.

Describe the solution you'd like Set a reasonable low but positive limit.

Describe alternatives you've considered None

LDD Dictionary Version 1.5.1.0

PDS4 IM Version 1.18.0.0

Need-by Date No pressing need.

Additional context DMSP follow-up.

rgdeen commented 2 years ago

Why set a low limit at all? What if you have a time-lapse of seasonal changes... once every 3 months is 1.28e-7 frames per second. Not unreasonable. So unless PDS limits can be "> x" rather than ">= x" then it should be set to 0. Even 0 is not unreasonable... consider a repeating observation that is in movie format but once happens to get just one frame... that's a single-frame movie where rate is undefined so 0 is the most reasonable value to put there. (you wouldn't want to convert it to a non-movie format just because it has only 1 frame, consistency is important in data set formatting).

acraugh commented 2 years ago

Ultimately the validation here just needs to be consistent with whatever is decided for the class generally in issue #203. They will both be addressed at the same time, so that should be easy to achieve.

thareUSGS commented 2 years ago

I will take a stab at this and simply define the constraint as positive number (min=0.0). In branch v1.6.0.0. https://github.com/pds-data-dictionaries/ldd-disp/tree/v1600

acraugh commented 2 years ago

@thareUSGS: Zero is not a positive number. If you look into the dmsp-test branch where the team has been working, you can see the modification for frame_rate we proposed. The initial testing behaved as expected in Oxygen. We're writing regression tests for various things now. I'll upload a few regression tests to the same branch as we get them ready to go. The tests need to be checked out in the repo anyway.

thareUSGS commented 2 years ago

Okay >=0.0. I like to option of allowing 0.0 and I added it in the description (v1.6.0.0). Great seeing all the tests - thanks! I will merge them into new branch once you think you are done.

acraugh commented 2 years ago

Then you need to define when it is valid to have a frame rate of zero and what that means. It makes no sense to me. Describing a 2D thing as a degenerate 3d thing is deceptive.

-Anne.

On Tue, Apr 26, 2022 at 11:57 AM Trent Hare @.***> wrote:

Okay >=0.0. I like to option of allowing 0.0 and I added it in the description (v1.6.0.0). Great seeing all the tests - thanks! I will merge them into new branch once you think you are done.

— Reply to this email directly, view it on GitHub https://github.com/pds-data-dictionaries/PDS4-LDD-Issue-Repo/issues/205#issuecomment-1109970932, or unsubscribe https://github.com/notifications/unsubscribe-auth/ADBV2QAH7V55RNROPC66ISLVHAG73ANCNFSM5UJM5GBQ . You are receiving this because you authored the thread.Message ID: @.*** com>

rgdeen commented 2 years ago

Degenerate cases are real and it is far better to treat them consistently than to have warts in the data set. If an instrument takes a movie you'd expect that to be in a movie format, for all such acquisitions. If it happens to be a single frame because of a fault or (questionable) command, that doesn't negate the fact that it's a movie just like any other movie the instrument acquires.

Consistency is important in a data set and that means accepting some degenerate cases when they come along.

I would not advocate for an instrument that takes both movies and stills to treat all stills as degenerate movies... but in the case where it really is a movie acquisition that ended up as a single frame, it should be treated as such. Which means frame rate = 0. (data providers could of course choose to put in a commanded or nominal frame rate, but 0 is perfectly valid here).

acraugh commented 2 years ago

Wouldn't it make sense to have a real frame rate consistent with the rest of the collection, then, rather than zero? Particularly if looping flags are set?

-Anne.

On Tue, Apr 26, 2022 at 1:07 PM rgdeen @.***> wrote:

Degenerate cases are real and it is far better to treat them consistently than to have warts in the data set. If an instrument takes a movie you'd expect that to be in a movie format, for all such acquisitions. If it happens to be a single frame because of a fault or (questionable) command, that doesn't negate the fact that it's a movie just like any other movie the instrument acquires.

Consistency is important in a data set and that means accepting some degenerate cases when they come along.

I would not advocate for an instrument that takes both movies and stills to treat all stills as degenerate movies... but in the case where it really is a movie acquisition that ended up as a single frame, it should be treated as such. Which means frame rate = 0. (data providers could of course choose to put in a commanded or nominal frame rate, but 0 is perfectly valid here).

— Reply to this email directly, view it on GitHub https://github.com/pds-data-dictionaries/PDS4-LDD-Issue-Repo/issues/205#issuecomment-1110044334, or unsubscribe https://github.com/notifications/unsubscribe-auth/ADBV2QCS6W7DE3NBHRNE2J3VHAPGPANCNFSM5UJM5GBQ . You are receiving this because you authored the thread.Message ID: @.*** com>

rgdeen commented 2 years ago

well as I said I'd probably put in the commanded rate. But if that's not available, and I could easily see scenarios where it's not, then what else would one put?

Bottom line though, I'm not aware that PDS even has the capability to specify > 0 instead of >= 0 so practically speaking, we kind of have to go with >= 0. The fact that there's a plausible scenario where 0 might actually make sense is a bonus.