-
Notifications
You must be signed in to change notification settings - Fork 0
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
SOOP TMV NRT - add attributes to CPHL and TURB variables in NetCDF #147
Conversation
Codecov Report
@@ Coverage Diff @@
## master #147 +/- ##
==========================================
+ Coverage 90.02% 90.03% +0.01%
==========================================
Files 40 40
Lines 2055 2058 +3
Branches 309 310 +1
==========================================
+ Hits 1850 1853 +3
Misses 120 120
Partials 85 85
Continue to review full report at Codecov.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good apart from one typo that needs to be fixed.
aodndata/soop/soop_tmv_nrt.py
Outdated
@@ -25,6 +25,16 @@ | |||
NC_JSON_TEMPLATE_MOORING = resource_filename("aodndata", "templates/soop_tmv_nrt_nc_template_mooring.json") | |||
NC_JSON_TEMPLATE_TRAJECTORY = resource_filename("aodndata", "templates/soop_tmv_nrt_nc_template_trajectory.json") | |||
|
|||
CHLU_PARAMS = { | |||
'blank': 55, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should be 40. See aodn/content#434 (comment)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In order to convert the counts, we decided to use the values here (at least, that's what I did):
https://github.com/aodn/imos-toolbox/blob/spirit/Preprocessing/spiritCountToEngPP.txt
chluBlank = 55
chluScale = 0.0123
turbBlank = 50
turbScale = 0.006
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, my bad. I now realise I used 40 for CPHL blank when I processed the spirit of tas delayed mode data and remember the reason was because otherwise I would end up with negative concentration in engineering units! I raised this problem to Randall Lee and asked him to comment on my decision to use 40 instead of 55 but he never came back to me.
For consistency I suppose I would be keen to keep using 40 for CPHL blank. One thing you could do when re-processing the whole archive is see what is the minimum count value that is found and hope it is no less than 40.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry, I'm really confused now. Are these values real and measured or eyeballed? Maybe it would be worse sending an email again to Randall (which I'm happy to do)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My understanding is that they're arbitrary. Strictly we should use coefficient calibrations that can change every time the instrument is serviced but we don't always have this information + even if we had it in practice, FLNTU sensors are such that these coef calibrations need to be adjusted a posteriori, when cross compared with analysis on water samples, in order to deliver best turbidity and chlorophyl. The same problem applies to FLNTUs on moorings. By default we always leave the manufacturer default coefficients applied although they're not ideal. The idea is to get out of the "count" world to get to the "engineering units" world so that we can then compare to water sample analysis and adjust when necessary.
Happy for you to contact randall.lee@epa.vic.gov.au (also include alastair.hirst@epa.vic.gov.au) in an attempt to clarify on this but in case it's too hard to get a response this shouldn't prevent us from moving forward.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
randall is investigating a bit more, but in the meantime, the values are
FLNTU #698 | |
---|---|
CHL-a = (FLcounts - FL blank)*FL_scale | |
FL_blank = 55 | |
FL_scale=0.0123 | |
NTU = (NTUcounts - NTU blank)*NTU_scale | |
NTU_blank = 50 | |
NTU_scale = 0.006 |
371eebb
to
c12792f
Compare
@bpasquer would you mind merging this please? |
No description provided.