From e6d6ccba7c48898a7e24ce20ff0b702be983370a Mon Sep 17 00:00:00 2001 From: abuts Date: Wed, 18 Dec 2024 14:15:33 +0000 Subject: [PATCH] Re #1790 subtle bug in serializable preventing overloading to_struct in case of recursion. --- .../@IX_detector_array/IX_detector_array.m | 5 + herbert_core/utilities/classes/@hashable/eq.m | 5 +- herbert_core/utilities/classes/@hashable/ne.m | 5 + .../@serializable/private/from_bare_struct_.m | 2 +- herbert_core/utilities/maths/equal_to_tol.m | 347 ++++-------------- horace_core/sqw/@Experiment/Experiment.m | 2 - 6 files changed, 90 insertions(+), 276 deletions(-) diff --git a/herbert_core/classes/instrument_classes/instrument/detectors/@IX_detector_array/IX_detector_array.m b/herbert_core/classes/instrument_classes/instrument/detectors/@IX_detector_array/IX_detector_array.m index af7a8b367e..39f15d32de 100644 --- a/herbert_core/classes/instrument_classes/instrument/detectors/@IX_detector_array/IX_detector_array.m +++ b/herbert_core/classes/instrument_classes/instrument/detectors/@IX_detector_array/IX_detector_array.m @@ -596,6 +596,11 @@ % Current version of class definition ver = 1; end + function flds = hashableFields(~) + % Return cellarray of properties defining the class hash for + % comparison + flds = {'det_bank', 'filename'}; + end function flds = saveableFields(~) % Return cellarray of properties defining the class diff --git a/herbert_core/utilities/classes/@hashable/eq.m b/herbert_core/utilities/classes/@hashable/eq.m index 1bbf8cd05f..824220e567 100644 --- a/herbert_core/utilities/classes/@hashable/eq.m +++ b/herbert_core/utilities/classes/@hashable/eq.m @@ -2,7 +2,6 @@ % Return a logical variable stating if two serializable objects are equal or not % % >> [iseq, mess] = eq (obj1, obj2) -% >> [iseq, mess] = eq (obj1, obj2, p1, p2, ...) % % Input: % ------ @@ -11,6 +10,10 @@ % obj2 Object on right-hand side % % See also equal_to_tol +if ~isa(obj2,class(obj1)) + iseq = false; + return; +end if ~all(size(obj1)==size(obj2)) iseq = false; return; diff --git a/herbert_core/utilities/classes/@hashable/ne.m b/herbert_core/utilities/classes/@hashable/ne.m index 5dd31e677b..4ab71ab0c3 100644 --- a/herbert_core/utilities/classes/@hashable/ne.m +++ b/herbert_core/utilities/classes/@hashable/ne.m @@ -12,6 +12,11 @@ % % See also equal_to_tol +% classes are different +if ~isa(obj2,class(obj1)) + isneq = true; + return; +end % sizes of objects are different if ~all(size(obj1)==size(obj2)) isneq = true; diff --git a/herbert_core/utilities/classes/@serializable/private/from_bare_struct_.m b/herbert_core/utilities/classes/@serializable/private/from_bare_struct_.m index 14b2038237..7a47a7a12d 100644 --- a/herbert_core/utilities/classes/@serializable/private/from_bare_struct_.m +++ b/herbert_core/utilities/classes/@serializable/private/from_bare_struct_.m @@ -104,7 +104,7 @@ if isstruct(val) if isfield(val,'serial_name') % Structure is one that has been created by to_struct - val = serializable.from_struct (val); + val = obj.from_struct (val); end end obj.(field_name) = val; diff --git a/herbert_core/utilities/maths/equal_to_tol.m b/herbert_core/utilities/maths/equal_to_tol.m index dcd487f7f3..7075cf8998 100644 --- a/herbert_core/utilities/maths/equal_to_tol.m +++ b/herbert_core/utilities/maths/equal_to_tol.m @@ -60,11 +60,6 @@ % % 'nan_equal' Treat NaNs as equal (true or false; default=true) % -% 'ignore_str' Ignore the length and content of strings or cell arrays -% of strings (true or false; default=false) -% -% throw_on_err Instead of returning error codes, thow error if some -% fields are not consistent % % 'name_a' Explicit name of variable a for use in messages % Usually not required, as the name of a variable will @@ -73,11 +68,19 @@ % discoverable in Matlab, and default 'input_1' will be % used unless a different value is given with the keyword % 'name_a'. -% % 'name_b' Explicit name of variable b for use in messages. % The same comments apply as for 'name_a' except the % default is 'input_2' +% 'ignore_str' Ignore the length and content of strings or cell arrays +% of strings (true or false; default=false) +% +% throw_on_err Instead of returning error codes, thow error if +% comparison returns false % +% Valid keys (if present, true, if absent, false) are: +% '-ignore_str' Ignore the length and content of strings or cell arrays +% '-throw_on_err' Instead of returning error codes, thow error if +% comparison returns false. % % Output: % ------- @@ -129,169 +132,14 @@ if isempty(name_b) name_b = name_b_default; end - % Lazy handling of MATLAB strings [a,b] = convertStringsToChars(a,b); - -% Parse input arguments -if nargin==2 - % Inputs to compare only; save an expensive call to parse_arguments - opt.nan_equal = true; - opt.ignore_str = false; - opt.tol = [0,0]; - opt.throw_on_err=false; - -elseif nargin==3 && isnumeric(varargin{1}) - % Case of no optional arguments; save an expensive call to parse_arguments - opt.nan_equal = true; - opt.ignore_str = false; - - % Determine if legacy input; it must be if tol is scalar - if isscalar(varargin{1}) - opt.tol=check_tol(varargin{1},0); - else - opt.tol=check_tol(varargin{1}); - end - opt.throw_on_err=false; -else - % Optional arguments must have been given; parse input arguments - % opt filled with default for new format; strip min_denominator away later - - opt = struct(... - 'tolerance',[],... - 'abstolerance',0,... - 'reltolerance',0,... - 'ignore_str',false,... - 'nan_equal',true,... - 'name_a',name_a,... - 'name_b',name_b,... - 'min_denominator',0,... - 'throw_on_err',false); - cntl.keys_once=false; % so name_a and name_b can be overridden by input arguments - cntl.keys_at_end=false; % as may have name_a or name_b appear first in some cases - [par, opt, present, ~] = parse_arguments(varargin, opt, cntl); - - % Determine the tolerance - ok_positive_scalar = @(x)(isnumeric(x) && isscalar(x) && ~isnan(x) && x>=0); - if numel(par)==1 && isnumeric(par{1}) - % There is a single parameter that is numeric, so must be tol - if isscalar(par{1}) - % Legacy format - tol=check_tol(par{1},opt.min_denominator); - else - % New format - tol=check_tol(par{1}); - % Invalid keyword 'min_denominator' cannot present with new format - if present.min_denominator - error('HERBERT:equal_to_tol:invalid_argument',... - '''min_denominator'' is only valid for legacy scalar tolerance') - end - end - % Check that tolerance has not been given as a keyword parameter as well - if present.tolerance || present.abstolerance || present.reltolerance - error('HERBERT:equal_to_tol:invalid_argument',... - ['Cannot give the tolerance as third input argument and'... - 'also as a keyword parameter']) - end - - elseif numel(par)==0 - % No tolerance parameter given, so determine from keywords if possible. - if ~any([present.tolerance, present.abstolerance, present.reltolerance]) - % No tolerance keywords present - use default tol; still unresolved - % if legacy or not, but presence of min_denominator will not make any - % difference to the test. Just need to check it is valid if given - if present.min_denominator - % Treat as legacy - tol = check_tol(0,opt.min_denominator); - else - % Treat as new format - tol = [0,0]; - end - - else - % Tolerance keyword(s) present; usage is therefore non-legacy. - - % Check that invalid keyword 'min_denominator' is not present - if present.min_denominator - error('HERBERT:equal_to_tol:invalid_argument',... - '''min_denominator'' is only valid for legacy argument format') - end - - % Determine tolerance - if present.tolerance && ~(present.abstolerance || present.reltolerance) - if isnumeric(opt.tolerance) - tol = check_tol(opt.tolerance); - else - error('HERBERT:equal_to_tol:invalid_argument',... - '''tol'' must be numeric') - end - elseif ~present.tolerance - if present.abstolerance && present.reltolerance - if ok_positive_scalar(opt.abstolerance) && ok_positive_scalar(opt.reltolerance) - tol = [opt.abstolerance,opt.reltolerance]; - else - error('HERBERT:equal_to_tol:invalid_argument',... - ['''abstol'' and ''reltol'' must both be '... - 'numeric scalars greater or equal to zero']) - end - elseif present.abstolerance - if ok_positive_scalar(opt.abstolerance) - tol = [opt.abstolerance,0]; - else - error('HERBERT:equal_to_tol:invalid_argument',... - '''abstol'' must be a numeric scalar greater or equal to zero') - end - elseif present.reltolerance - if ok_positive_scalar(opt.reltolerance) - tol = [0,opt.reltolerance]; - else - error('HERBERT:equal_to_tol:invalid_argument',... - '''reltol'' must be a numeric scalar greater or equal to zero') - end - else - tol = [0,0]; - end - else - error('HERBERT:equal_to_tol:invalid_argument',... - '''tol'' cannot be present with ''abstol'' or ''reltol''') - end - - end - else - error('HERBERT:equal_to_tol:invalid_argument',... - 'Unrecognized input argument(s): %s',disp2str(par)); - end - - % Strip away temporary fields - name_a = opt.name_a; - name_b = opt.name_b; - if isempty(name_a) - name_a = name_a_default; - end - if isempty(name_b) - name_b = name_b_default; - end - opt = rmfield(opt, {'min_denominator','name_a','name_b',... - 'tolerance','abstolerance','reltolerance'}); - - if islognumscalar(opt.ignore_str) - opt.ignore_str = logical(opt.ignore_str); - else - error('HERBERT:equal_to_tol:invalid_argument',... - 'Check ''ignore_str'' is logical scalar (or 0 or 1)') - end - if islognumscalar(opt.nan_equal) - opt.nan_equal = logical(opt.nan_equal); - else - error('HERBERT:equal_to_tol:invalid_argument',... - 'Check ''nan_equal'' is logical scalar (or 0 or 1)') - end - opt.tol = tol; -end - +% +opt = parse_equal_to_tol_inputs(name_a,name_b,varargin{:}); +% % Now perform comparison try - equal_to_tol_private(a,b,opt,name_a,name_b); + equal_to_tol_private(a,b,opt); catch ME if opt.throw_on_err rethrow(ME) @@ -306,52 +154,7 @@ end %-------------------------------------------------------------------------------------------------- -function tol_out = check_tol (tol, min_denominator) -% Convert all the possible inputs into [abs_tol, rel_tol] -% Assumes tol_in is numeric -% -% >> tol = check_tol (tol_in) -% >> tol = check_tol (tol_in, min_denominator] - - -ok_positive_scalar = @(x)(isnumeric(x) && isscalar(x) && ~isnan(x) && x>=0); - -if isempty(tol) - tol_out = [0,0]; - -elseif isscalar(tol) - if ~isnan(tol) - if tol>=0 - tol_out = [tol,0]; - else - if ok_positive_scalar(min_denominator) - tol_out = [min_denominator*abs(tol),abs(tol)]; - else - error('HERBERT:equal_to_tol:invalid_argument',... - 'Check value of ''min_denominator'' is greater or equal to zero'); - end - end - else - error('HERBERT:equal_to_tol:invalid_argument',... - 'Tolerance cannot be NaN'); - end - -elseif numel(tol)==2 - if all(tol>=0) - tol_out = tol; - else - error('HERBERT:equal_to_tol:invalid_argument',... - 'Check tolerance has form [abs_tol, rel_tol] where both are >=0'); - end -else - error('HERBERT:equal_to_tol:invalid_argument',... - 'The tolerance is not a positive numeric scalar'); -end - -end - -%-------------------------------------------------------------------------------------------------- -function equal_to_tol_private(a,b,opt,name_a,name_b) +function equal_to_tol_private(a,b,opt) % Check the equality of two arguments within defined tolerance % Used by public functions equal_to_tol and equaln_to_tol % @@ -372,7 +175,7 @@ function equal_to_tol_private(a,b,opt,name_a,name_b) % name_b Name of second variable -if opt.ignore_str && (iscellstr(a)||ischar(a)) && (iscellstr(b)||ischar(b)) +if opt.ignore_str && (iscellstr(a)||istext(a)) && (iscellstr(b)||istext(b)) % Case of strings and cell array of strings if they are to be ignored % If cell arrays of strings then contents and number of strings are % ignored e.g. {'Hello','Mr'}, {'Dog'} and '' are all considered equal @@ -387,32 +190,15 @@ function equal_to_tol_private(a,b,opt,name_a,name_b) if ~isequal(sz,size(b)) error('HERBERT:equal_to_tol:inputs_mismatch',... '%s and %s: Sizes of arrays of objects being compared are not equal',... - name_a,name_b); + opt.name_a,opt.name_b); end - if ismethod(a,'eq') && ~isa(a, 'PixelDataBase') - try - opt.name_a = name_a; - opt.name_b = name_b; - opt = [fieldnames(opt),struct2cell(opt)]'; - opt = opt(:); - [is,mess] = eq(a,b,opt{:}); - catch ME - if strcmp(ME.identifier,'MATLAB:TooManyInputs') ||... - strcmp(ME.identifier,'MATLAB:TooManyOutputs') || ... - strcmp(ME.identifier,'MATLAB:UndefinedFunction') || ... - strcmp(ME.identifier,'MATLAB:maxlhs') - is = eq(a,b); - if ~is - mess = 'class "eq" operation returned false'; - end - else - rethrow(ME); - end - end - if ~is + if ismethod(a,'equal_to_tol') + [ok,mess] = equal_to_tol(a,b,opt); + if ~ok error('HERBERT:equal_to_tol:inputs_mismatch',... - 'Input object %s differs from input object %s reason: %s',... - name_a,name_b,mess); + ['%s and %s: Objects are not equal due to class-specific equal_to_tol method\n',... + ' Reason: %s'],... + opt.name_a,opt.name_b,mess); end return; end @@ -439,23 +225,28 @@ function equal_to_tol_private(a,b,opt,name_a,name_b) if ~isempty(extraA) || ~isempty(extraB) error('HERBERT:equal_to_tol:inputs_mismatch',... 'Input %s with extra fields: "%s" DIFFERENT from Input %s: with extra fields: "%s"',... - name_a,strjoin(extraA,'; '),name_b,strjoin(extraB,'; ')); + opt.name_a,strjoin(extraA,'; '),opt.name_b,strjoin(extraB,'; ')); end if isscalar(a) || isa(a, 'containers.Map') - name_a_ind = name_a; - name_b_ind = name_b; + name_a_ind = opt.name_a; + name_b_ind = opt.name_b; Sa = struct(a); Sb = struct(b); % If we get here, we need the "right" names fieldsA = fieldnames(Sa); for j=1:numel(fieldsA) - equal_to_tol_private(Sa.(fieldsA{j}), Sb.(fieldsA{j}), opt,... - [name_a_ind,'.',fieldsA{j}], [name_b_ind,'.',fieldsA{j}]); + lopt = opt; + lopt.name_a = [name_a_ind,'.',fieldsA{j}]; + lopt.name_b = [name_b_ind,'.',fieldsA{j}]; + equal_to_tol_private(Sa.(fieldsA{j}), Sb.(fieldsA{j}), lopt); end else + name_a = opt.name_a; + name_b = opt.name_b; for i=1:numel(a) + name_a_ind = [name_a,'(',arraystr(sz,i),')']; name_b_ind = [name_b,'(',arraystr(sz,i),')']; Sa = struct(a(i)); @@ -464,8 +255,11 @@ function equal_to_tol_private(a,b,opt,name_a,name_b) % If we get here, we need the "right" names fieldsA = fieldnames(Sa); for j=1:numel(fieldsA) - equal_to_tol_private(Sa.(fieldsA{j}), Sb.(fieldsA{j}), opt,... - [name_a_ind,'.',fieldsA{j}], [name_b_ind,'.',fieldsA{j}]); + lopt = opt; + lopt.name_a = [name_a_ind,'.',fieldsA{j}]; + lopt.name_b = [name_b_ind,'.',fieldsA{j}]; + + equal_to_tol_private(Sa.(fieldsA{j}), Sb.(fieldsA{j}), lopt); end end end @@ -479,7 +273,7 @@ function equal_to_tol_private(a,b,opt,name_a,name_b) if ~isequal(sz,size(b)) error('HERBERT:equal_to_tol:inputs_mismatch',... '%s and %s: Sizes of arrays of structures being compared are not equal',... - name_a,name_b); + opt.name_a,opt.name_b); end % Check fieldnames are identical @@ -491,18 +285,20 @@ function equal_to_tol_private(a,b,opt,name_a,name_b) if ~isempty(extraA) || ~isempty(extraB) error('HERBERT:equal_to_tol:inputs_mismatch',... 'The structure: "%s" names: "%s" DIFFER from the struct: "%s" names: "%s"',... - name_a,strjoin(extraA,'; '),name_b,strjoin(extraB,'; ')); + opt.name_a,strjoin(extraA,'; '),opt.name_b,strjoin(extraB,'; ')); end + name_a = opt.name_a; + name_b = opt.name_b; % Check contents of each field are the same if isscalar(a) - name_a_ind = name_a; - name_b_ind = name_b; for j=1:numel(fieldsA) - equal_to_tol_private (a.(fieldsA{j}),... - b.(fieldsA{j}), opt,... - [name_a_ind,'.',fieldsA{j}], [name_b_ind,'.',fieldsA{j}]); + lopt = opt; + lopt.name_a = [name_a,'.',fieldsA{j}]; + lopt.name_b = [name_b,'.',fieldsA{j}]; + + equal_to_tol_private (a.(fieldsA{j}),b.(fieldsA{j}), lopt); end else @@ -510,9 +306,12 @@ function equal_to_tol_private(a,b,opt,name_a,name_b) name_a_ind = [name_a,'(',arraystr(sz,i),')']; name_b_ind = [name_b,'(',arraystr(sz,i),')']; for j=1:numel(fieldsA) + lopt = opt; + lopt.name_a = [name_a_ind,'.',fieldsA{j}]; + lopt.name_b = [name_b_ind,'.',fieldsA{j}]; + equal_to_tol_private (a(i).(fieldsA{j}),... - b(i).(fieldsA{j}), opt,... - [name_a_ind,'.',fieldsA{j}], [name_b_ind,'.',fieldsA{j}]); + b(i).(fieldsA{j}), lopt); end end end @@ -526,21 +325,25 @@ function equal_to_tol_private(a,b,opt,name_a,name_b) if ~isequal(sz,size(b)) error('HERBERT:equal_to_tol:inputs_mismatch',... '%s and %s: Sizes of cell arrays being compared are not equal',... - name_a,name_b); + opt.name_a,opt.name_b); end + name_a = opt.name_a; + name_b = opt.name_b; % Check contents of each element of the arrays for i=1:numel(a) - name_a_ind = [name_a,'{',arraystr(sz,i),'}']; - name_b_ind = [name_b,'{',arraystr(sz,i),'}']; - equal_to_tol_private (a{i} ,b{i}, opt, name_a_ind, name_b_ind); + lopt = opt; + lopt.name_a =[name_a,'{',arraystr(sz,i),'}']; + lopt.name_b = [name_b,'{',arraystr(sz,i),'}']; + + equal_to_tol_private (a{i} ,b{i}, lopt); end elseif isnumeric(a) && isnumeric(b) % --------------------------------- % Both arguments are numeric arrays % --------------------------------- - equal_to_tol_numeric(a,b,opt.tol,opt.nan_equal,name_a,name_b); + equal_to_tol_numeric(a,b,opt); elseif ischar(a) && ischar(b) % ----------------------------------- @@ -550,10 +353,10 @@ function equal_to_tol_private(a,b,opt,name_a,name_b) if ~isequal(size(a),size(b)) error('HERBERT:equal_to_tol:inputs_mismatch',... '%s and %s: Sizes of character arrays being compared are not equal',... - name_a,name_b); + opt.name_a,opt.name_b); end - if isrow(a) && ~isempty(regexp(a, '\d{4}-\d{2}-\d{2}T\d{2}:\d{2}:\d{2}')) + if isrow(a) && ~isempty(regexp(a, '\d{4}-\d{2}-\d{2}T\d{2}:\d{2}:\d{2}','once')) % Probably dealing with datetime string try a = main_header_cl.convert_datetime_from_str(a); @@ -569,7 +372,7 @@ function equal_to_tol_private(a,b,opt,name_a,name_b) if ~strcmp(a,b) error('HERBERT:equal_to_tol:inputs_mismatch',... '%s and %s: Character arrays being compared are not equal',... - name_a,name_b); + opt.name_a,opt.name_b); end elseif strcmp(class(a),class(b)) @@ -580,32 +383,35 @@ function equal_to_tol_private(a,b,opt,name_a,name_b) if ~isequal(size(a),size(b)) error('HERBERT:equal_to_tol:inputs_mismatch',... '%s and %s: Sizes of arrays of objects being compared are not equal',... - name_a,name_b); + opt.name_a,opt.name_b); end if ~isequal(a,b) error('HERBERT:equal_to_tol:inputs_mismatch',... '%s and %s: Object (or object arrays) are not equal',... - name_a,name_b); + opt.name_a,opt.name_b); end - else % ----------------------------------------------- % Items being compared do not have the same class % ----------------------------------------------- error('HERBERT:equal_to_tol:inputs_mismatch',... '%s and %s: Have different classes: %s and %s',... - name_a,name_b,class(a),class(b)); + opt.name_a,opt.name_b,class(a),class(b)); end end %-------------------------------------------------------------------------------------------------- -function equal_to_tol_numeric(a,b,tol,nan_equal,name_a,name_b) +function equal_to_tol_numeric(a,b,opt) % Check two arrays have smae size and each element is the same within % requested relative or absolute tolerance. +tol = opt.tol; +nan_equal = opt.nan_equal; +name_a = opt.name_a; +name_b = opt.name_b; sz=size(a); if any(sz ~= size(b)) @@ -704,7 +510,6 @@ function equal_to_tol_numeric(a,b,tol,nan_equal,name_a,name_b) end else - diff = abs(a-b); [max_delta_abs, ind_abs] = max(diff); [max_delta_rel, ind_rel] = max(diff./max(abs(a),abs(b))); @@ -734,17 +539,15 @@ function equal_to_tol_numeric(a,b,tol,nan_equal,name_a,name_b) else ind=cell(1,numel(sz)); [ind{:}]=ind2sub(sz,i); - str=''; - for j=1:numel(ind) - str=[str,num2str(ind{j}),',']; - end - str=str(1:end-1); + j=1:numel(ind); + str=arrayfun(@(j)num2str(ind{j}),j,'UniformOutput',false); + str = strjoin(str,','); end end function rel = rel_diff(a, b) - rel = abs(a-b)./max(abs(a),abs(b)); +rel = abs(a-b)./max(abs(a),abs(b)); end diff --git a/horace_core/sqw/@Experiment/Experiment.m b/horace_core/sqw/@Experiment/Experiment.m index cda7c7ea21..93b945f8b0 100644 --- a/horace_core/sqw/@Experiment/Experiment.m +++ b/horace_core/sqw/@Experiment/Experiment.m @@ -96,8 +96,6 @@ % % Each argument can be a single object or array of objects. - obj = obj@serializable(); - % initialising the compressed component containers. % these may be overwritten if they are passed in as arguments % below