Skip to content

Add cps flow edits - #185

Open
eliascapriles-NOAA wants to merge 11 commits into
echostack-org:mainfrom
eliascapriles-NOAA:Add_cps_flow_edits
Open

Add cps flow edits#185
eliascapriles-NOAA wants to merge 11 commits into
echostack-org:mainfrom
eliascapriles-NOAA:Add_cps_flow_edits

Conversation

@eliascapriles-NOAA

Copy link
Copy Markdown
Contributor

Main edits:

  • adding convolution step before noise removal
  • convolving above and below the seafloor
  • adjusting the blackwell algorithm to use thresholds

@LOCEANlloydizard LOCEANlloydizard left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

thx @eliascapriles-NOAA! a few things I noticed while going through the changes:

  • we should we keep the parameters recipe-driven and avoid hard-coding the Blackwell/CPS values in the flow (channel=1, q=0.95, -50, wtheta=7, wphi=7, etc.)? If these are new parameters we want to use, I think it would be better to expose them in the flow and set them through the deployment recipe, so the processing stays configurable!

  • could we also preserve the original calibrated ds["Sv"] and store the convolved/processed field separately? That way the final mask can still be applied back to the original Sv

  • after the convolution and background-noise removal, I think sv_for_cps should be rebuilt from the processed variable. At the moment it looks like it may still refer to the version created before those steps?

Maybe you could also try deploying/testing this version directly using the CPS deployment recipe in echodataflow-recipes, together with the deployment instructions in the README. That should make it easier to check whether you're happy with the resulting CPS output? cheers!

@eliascapriles-NOAA

Copy link
Copy Markdown
Contributor Author

Hey @LOCEANlloydizard totally agree with these changes let me get on that, and fix the problems up. I wanted to crank out a quick solution at first, but let me get on fixing those up !

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