-
Notifications
You must be signed in to change notification settings - Fork 19
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
Detect_edits2 #13
base: master
Are you sure you want to change the base?
Detect_edits2 #13
Conversation
I had to modify the code in event_line and mean_and_trend as I found some errors after I changed detect. Note that the mean_and_trend function still needs the Stats toolbox to function, as you use fitlm. We would have to change the function to polyfit to get rid of the toolbox dependency.
|
||
[vEvent, vColor,vAlpha,vLineWidth,vFontSize]... | ||
= internal.stats.parseArgs(paramNames, defaults, varargin{:}); |
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.
Once again, this only works if the user has the Stats toolbox. I've removed it similarly to detect, so that users can use it without the toolbox
|
||
|
||
temp_here=squeeze(temp(loc(1),loc(2),:)); | ||
|
||
MHW=MHW{:,:}; | ||
MHW=MHW(MHW(:,8)==loc(1) & MHW(:,9)==loc(2),:); |
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.
Adding peak_date as an output of the detect function, there is one more dimension into the table, to the numbers had to be modified accordingly
period_plot=datenum(data_start,1,1):1:(datenum(data_start,1,1)+size(temp,3)-1); | ||
period_mhw=[datenum(num2str(MHW(:,1)),'yyyymmdd') datenum(num2str(MHW(:,2)),'yyyymmdd')]; | ||
period_mhw=[MHW(:,1) MHW(:,2)]; |
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.
For some reason, this gave me an error so I modified it
@@ -42,27 +42,39 @@ | |||
% | |||
% p_metric - A 2D numeric matrix (m-by-n) containing p value of | |||
% TREND_METRIC. | |||
vMetric = 'Frequency'; |
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.
mean_and_trend.m had the same toolbox issue.
@@ -50,6 +50,9 @@ | |||
% mhw_end - A numeric value in format of datennum(yyyy,mm,dd) indicating the end year for the period across | |||
% which MHW/MCS events are detected. | |||
% | |||
% mhw_peak - A numeric value in format of datennum(yyyy,mm,dd) indicating the date of maximum intensity for the period across | |||
% which MHW/MCS events are detected. | |||
% |
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.
Added mhw_peak for both MHW and MCS algorithms
'''Event'''); | ||
PercentileNames={'matlab','python'}; | ||
vpercentile=internal.stats.getParamVal(vpercentile,PercentileNames,... | ||
'''matlab'''); |
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.
Modified the input variables parsing so that users don't need the Stats toolbox
@@ -207,7 +222,7 @@ | |||
date_mhw(:,1)=2000; | |||
indextocal = day(datetime(date_mhw),'dayofyear'); | |||
|
|||
ts=str2double(string(datestr(mhw_start:mhw_end,'YYYYmmdd'))); | |||
ts= mhw_start:mhw_end; | |||
|
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.
Because mhw dates are now in datenums, coding can be simplified
'variablenames',{'mhw_onset','mhw_end','mhw_dur','int_max','int_mean','int_var','int_cum','xloc','yloc'}); | ||
|
||
if ~isempty(MHW) | ||
MHW=table(MHW(:,1),MHW(:,2),MHW(:,3),MHW(:,4),MHW(:,5),MHW(:,6),MHW(:,7),MHW(:,8),MHW(:,9),MHW(:,10),... |
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.
Indexing numbers had to be modified due to the addition of a variable.
I had to modify the code in event_line and mean_and_trend as I found some errors after I changed detect.
Note that the mean_and_trend function still needs the Stats toolbox to function, as you use fitlm. We would have to change the function to polyfit to get rid of the toolbox dependency.