smnorris / bcfishpass

Model and monitor aquatic habitat connectivity in BC. Tools to plan and prioritize the assessment and remediation of barriers.
https://smnorris.github.io/bcfishpass
Apache License 2.0
8 stars 13 forks source link

PSCIS barrier status fix table usage #521

Open smnorris opened 5 months ago

smnorris commented 5 months ago

Something is required to indicate why the barrier status in bcfishpass data does not match the barrier status in PSCIS. Easiest fix is to add the notes column from bcfishpass.user_pscis_barrier_status to various crossing views / data dumps, calling it something like pscis_barrier_status_adjustment_notes or similar.

See https://github.com/smnorris/bcfishpass/pull/519#issuecomment-2166395384

smnorris commented 5 months ago

Based on @NewGraphEnvironment comments, just adding a notes column to bcfishpass tables may not be enough, we really don't want PSCIS and bcfishpass barrier status to diverge - things get confusing and office review shouldn't totally overwrite field data unless we establish a process/criteria for this so all users are aware where barrier status is getting adjusted.

Reporting is a different matter though, it is useful/necessary to exclude or add certain locations. Switching the barrier status in bcfishpass.crossings_wcrp_vw to adjust this reporting based on these fixes makes sense.

Unfortunately this isn't straightforward - various stats are generated based on barrier status of crossings. bcfishpass.crossings_wcrp_vw would have to be re-worked slightly and various upstream summaries re-calculated based on the barrier status in this table rather than the primary crossings table. Probably not difficult to apply but adds another layer of processing when refreshing data.

smnorris commented 5 months ago

@NewGraphEnvironment @captainmarmot

I've been thinking a bit more about how to deal with this.

I see there are also 4 fixes in this table from New Graph - 3 in PARS and 1 in MORR, from Mateo, based on imagery review. https://github.com/smnorris/bcfishpass/blob/main/data/user_pscis_barrier_status.csv#L1305 I haven't looked into what is going on at these crossings but I presume the fixes are appropriate - PSCIS assessments at those locations are either long out of date or have some other issues.

So - I agree that we don't generally want to overwrite PSCIS barrier status. But it is necessary at times - PSCIS data can be dated or incorrect.

My proposal:

My preference is that bcfishpass pulls from the best sources to generate modelling rather than be a source itself. Fixes to source data like PSCIS/CABD/FISS are useful and welcome - but these fixes to data sources should not live directly in this repository.

smnorris commented 4 months ago

https://github.com/smnorris/bcfishpass/pull/529 includes more updates to PSCIS, particularly setting more fords as passable. This makes sense for reporting and is just in the Elk where we have already done this for some time - I'm fine with merging as is.

But I'd like to see this issue resolved so we have a documented process for adjusting PSCIS that works for all users.

nickw-CWF commented 4 months ago

@smnorris I know the others will have to chime in on the overall question of how to handle fixes to PSCIS barrier statuses. From CWF's perspective, we would be fine having the fixes only be applied to crossings_wcrp_vw. We don't even necessarily have to overwrite the 'barrier_status' field that comes from PSCIS, we could add a new field (e.g., 'wcrp_barrier_status') that pulls the 'barrier_status' field values by default and applies changes from the fix table where they exist. That we can clearly see which points differ. Having a notes column that pulls from the fix table would probably be helpful as well.

smnorris commented 4 months ago

Thanks @nickw-CWF

There obviously won't always be a clear line as to which changes are best just for reporting and which should be merged into bcfishpass.crossings_vw (and downstream products) but hopefully we can put together a rough guide.

NewGraphEnvironment commented 4 months ago

I see there are also 4 fixes in this table from New Graph - 3 in PARS and 1 in MORR, from Mateo, based on imagery review.

Wasn't aware this was happening I don't think... can they just go in data/user_modelled_crossing_fixes.csv as fixes linked to only the modelled_crossing_id such as the one below and we leave the PSCIS corrections alone?

https://github.com/smnorris/bcfishpass/blob/3f260ec6d885e302f4f94d6a7f2dc51dc67fb2e8/data/user_modelled_crossing_fixes.csv#L6163

I have to reiterate that I think that the labelling FORDs as passable in the csvs is not necessary since the modelling has us covered no? Maybe right now barrier_result = other is being interpreted as passable but perhaps crossing_subtype = ford could be linked to "passable" barrier status? Sorry if that isn't correct or clear.

Also, IMO- using bcfishpass as a place to put anecdotal information and modify model outputs with it is perhaps a bad idea too. If we try to use standardized methods in bcfishpass and build custom interpretations (ie. "that crossing might be a barrier sometimes based on local knowledge") outside of this software - the tools are more objective, will persist with credibility longer into the future and will be better suited to a wider user base.

smnorris commented 4 months ago

I see there are also 4 fixes in this table from New Graph - 3 in PARS and 1 in MORR, from Mateo, based on imagery review.

Wasn't aware this was happening I don't think... can they just go in data/user_modelled_crossing_fixes.csv as fixes linked to only the modelled_crossing_id such as the one below and we leave the PSCIS corrections alone?

https://github.com/smnorris/bcfishpass/blob/3f260ec6d885e302f4f94d6a7f2dc51dc67fb2e8/data/user_modelled_crossing_fixes.csv#L6163

The fix/adjustment won't show up in bcfishpass.crossings_vw that way - we pull from PSCIS (and the fix table where applicable) as first priority - a fix in the modelled crossing fix table won't show in that output. So if user wants reporting to reflect passable structures at those points we'd have to apply the fix some other way.

I have to reiterate that I think that the labelling FORDs as passable in the csvs is not necessary since the modelling has us covered no? Maybe right now barrier_result = other is being interpreted as passable but perhaps crossing_subtype = ford could be linked to "passable" barrier status? Sorry if that isn't correct or clear.

You're right - crossings with status UNKNOWN (all PSCIS fords) won't be included in reporting: https://github.com/smnorris/bcfishpass/blob/main/model/01_access/sql/barriers_anthropogenic.sql#L34, I think any upstream stats for these crossings will presume the crossing is passable.

Also, IMO- using bcfishpass as a place to put anecdotal information and modify model outputs with it is perhaps a bad idea too. If we try to use standardized methods in bcfishpass and build custom interpretations (ie. "that crossing might be a barrier sometimes based on local knowledge") outside of this software - the tools are more objective, will persist with credibility longer into the future and will be better suited to a wider user base.

Agreed, with the caveat that the local knowledge is critical and needs to be available to all users. If a 2013 PSCIS assessement is known to have been replaced with a bridge in 2018 or so everyone needs that info until it is reassessed (which may never happen). bcfishpass is the only place to put this right now, hence my suggestion that some PSCIS fixes/adjustments could live in the PSCIS fixes repo (and be administered by the province).

smnorris commented 4 months ago

Based on feedback from @NewGraphEnvironment and @captainmarmot I am pulling the majority of these WCRP specific PSCIS barrier status adjustments over to a WCRP specific table, and will add some documentation on when it is appropriate to add a record to this master fix table. I think the only scenarios would be:

@nickw-CWF Above is straightforward but I notice that of the 1354 records in the PSCIS fix table, 488 are redundant - the barrier status in the fix table matches the latest PSCIS assessment status. All are from 2021-01-01, from the four WCRPs at the time. It looks like a few of them by you and me were in advance of PSCIS submissions but the reason for the rest is unclear. I'll remove unless you want to retain?

select count(*)
from bcfishpass.user_pscis_barrier_status u
inner join bcfishpass.pscis p on u.stream_crossing_id = p.stream_crossing_id
where u.user_barrier_status = current_barrier_result_code;

 count 
-------
   488
smnorris commented 4 months ago

I'm going to leave things as they at the moment and concentrate on merging/deploying v0.5.0. Once that is done, I'll:

nickw-CWF commented 4 months ago

Based on feedback from @NewGraphEnvironment and @CaptainMarmot I am pulling the majority of these WCRP specific PSCIS barrier status adjustments over to a WCRP specific table, and will add some documentation on when it is appropriate to add a record to this master fix table. I think the only scenarios would be:

  • support reporting in advance of a PSCIS submission (reassessment fieldwork complete but data is not yet available)
  • imagery clearly shows a remediation (or - in theory - a flood, landslide, river path change etc) at a PSCIS site but no reassessment is planned

One additional case that I can think of is that there have been occasions where the structure type (e.g., CBS, OBS) set in a PSCIS assessment conflicts with what can be seen in the photos from the assessment. Is this an update the could be fixed in PSCIS itself and/or would it be the type of fix we want to have applied to bcfishpass.crossings_vw and downstream products (rather than just bcfishpass.crossings_wcrp_vw?

smnorris commented 4 months ago

Yes, I think that would be a PSCIS fix - the photo and structure type should match unless there are several structures at the same crossing.

Note PSCIS issues for fixing here https://github.com/smnorris/PSCIS_datafixes.

nickw-CWF commented 4 months ago

Yes, I think that would be a PSCIS fix - the photo and structure type should match unless there are several structures at the same crossing.

Note PSCIS issues for fixing here https://github.com/smnorris/PSCIS_datafixes.

This makes sense to me, and I'm glad we now have a way to flag these updates for PSCIS itself. I guess my only concern is how quickly these updates will actually be reflected in PSCIS. Do you know what the schedule is for review/implementation of the fixes?

smnorris commented 4 months ago

No schedule - but I'm on contract with BC and we've started making some requests for access to the db, @captainmarmot will have more insight on scheduling. In the past, submissions of fixes has been straightforward - BC can ingest fix scripts without issue.

What slows the process for me is:

  1. creating a dev/test oracle environment is awkward and time consuming, it is a new process each time
  2. each individual fix can take a lot of research time to determine the best resolution

If we expect fixes to be more regular, 1 can likely be resolved. I'd love to delegate some of 2 but the right fix is not apparent without knowledge of internal PSCIS db rules. Best I can do is ask that any fixes requested have a lot of detail, making it easier to determine the best resolution.

CaptainMarmot commented 4 months ago

I'm happy to have this as one of the priorities for your contract time Simon. Some housekeeping in PSCIS is certainly overdue. I'm also supportive of the proposed fix repository and creating some documentation of what is appropriate to be included.