ioos / ioos_qc

:ballot_box_with_check: :ocean: IOOS QARTOD and other Quality Control tests implemented in Python
https://ioos.github.io/ioos_qc/
Apache License 2.0
46 stars 27 forks source link

QARTOD.Location_test: Add a range from target feature #46

Open JessyBarrette opened 3 years ago

JessyBarrette commented 3 years ago

The present pull request is adding three optional inputs to the location_test that define a target location (target_lon [degree_east], target_lat [degree_north]) and an acceptable distance from the target (target_range [m]). _targetlon and _targetlat can either be a single value or an array the same size as the lon and lat inputs.

The location_test output a SUSPECT flag if the distance from the target is exceeding the provided target_range.

As an example, such location_test can be useful to test:

Although not necessarily considered to be used that way, it could also potentially be used to compare a mobile platform trajectory versus a predefined trajectory. A new tool distance_from_target is also added to the ioos_qc.utils module to compute the distance between two locations.

A series of tests were also added to the test_qartod module to test those new inputs.

kwilcox commented 3 years ago

Thanks @JessyBarrette! The doc-string of the location_test method could use a cleanup now. It is essentially doing three different tests now, correct?

It might make sense to separate these into three different tests and the result of the location_test be the aggregation of them? Maybe the user can flag which location tests they would like to use? Going to pass this through some other people but interested in your thoughts as well!

JessyBarrette commented 3 years ago

@kwilcox You're right about the different tests, yes!

Regarding how to structure the different tests. I think separating them as you suggest would make a lot of sense. If only one of the tests can be run at the time and the different tests are aggregated after, it can definitely make it easier to identify each test result, troubleshoot issues, and just make it cleaner to handle.

Another point maybe, I think each of those tests should have the potential to apply either a SUSPECT and/or FAIL result and leave it to the user to decide which one is best for them to apply.

kwilcox commented 3 years ago

@JessyBarrette I'm back on ioos_qc for a few days next week and will get to this!

JessyBarrette commented 3 years ago

@kwilcox sounds good we're all scrambling these days. I figured you were busy

ocefpaf commented 5 months ago

@JessyBarrette sadly Kyle is no longer working on this and we ar trying to revisit the open PRs in ioos_qc. It seems that the only missing issue here is how to organize the tests, right? Is this something you are still wiling to work on?

PS: I'm not super familiar with the QARTOD manuals. Do you mind posting the manual/page this test is from so we can document it.

JessyBarrette commented 5 months ago

This is pretty old haha. From page 16 of the Manual for Real-Time Quality Control of In-situ Temperature and Salinity Data

Screenshot 2024-03-18 at 9 56 49 AM

I realize now this optional test do not match the description of the QARTOD Location test.

I do see the need for such test in some particular cases (ex: Buoy drifting from an initial target mooring location). But potentially this should not live within the QARTOD module.

I am open to the idea of either dropping this PR or moving this test to another module.

ocefpaf commented 5 months ago

I do see the need for such test in some particular cases (ex: Buoy drifting from an initial target mooring location). But potentially this should not live within the QARTOD module.

I'll leave that to the new dev @iwensu0313. But I agree, we should not overload the qartod module with it and it is a valuable test to have somewhere.

iwensu0313 commented 5 months ago

@JessyBarrette I like your new test ideas as well. I think if we want to make amendments to the QARTOD tests, we would want to propose this to the IOOS community. @ocefpaf / IOOS shared that the manuals are moving to github now (qartod_manuals), which might make it easier to collect feedback and update the manuals more frequently moving forward.

I am only familiar w/ the QARTOD implementation in ioos_qc so far. But looks like there are many other types of QC modules in ioos_qc. I see that a couple of them are based on 'ocean observing type' (e.g. gliders, argo). Will have to take a look at how the modules are organized currently to see what makes the most sense. In the meantime, feel free to propose a good location for this.

Maybe @lukecampbell you have some immediate thoughts?