Skip to content

agg_check_parfor_chunk: add rev10 orchestration cleanup plus real-input validation and profiling harnesses#18

Open
nicklasorte wants to merge 1 commit intomainfrom
codex/implement-strategy-1-for-agg_check_parfor_chunk
Open

agg_check_parfor_chunk: add rev10 orchestration cleanup plus real-input validation and profiling harnesses#18
nicklasorte wants to merge 1 commit intomainfrom
codex/implement-strategy-1-for-agg_check_parfor_chunk

Conversation

@nicklasorte
Copy link
Copy Markdown
Owner

Motivation

  • Unblock safe production use of the agg-check orchestration by removing hard debug pauses while preserving existing chunking and checkpoint semantics.
  • Preserve output contract and disk-based checkpoint model while assuming the downstream monte_carlo_Pr_dBm_rev3_app will be supplied separately.

Description

  • Added agg_check_parfor_chunk_rev10_app.m which was cloned from rev9, keeps the same signature and outputs, documents the Strategy-1 scope, and removes interactive pause stops in the production path while replacing critical NaN checks with fail-closed error(...) guards.
  • Kept chunk scheduling, parfor orchestration, save/load retry loops, and post-assembly cleanup unchanged to retain the existing persistent checkpoint behavior.
  • Added validate_agg_check_parfor_chunk_rev9_rev10_real.m which runs rev9 and rev10 on identical inputs, measures runtimes, computes speedup, performs deep comparisons (numeric/cell/struct with NaN/Inf pattern checks), and fails closed when drift exceeds tolerances.
  • Added profile_agg_check_parfor_chunk_rev9_real.m which profiles agg_check_parfor_chunk_rev9_app on real inputs, reports top contributors, buckets time into orchestration vs downstream compute vs disk activity, and returns a results struct.

Testing

  • Performed automated static checks on the new files including function-listing and pattern searches with rg to confirm presence of expected functions and removal of interactive pauses.
  • Confirmed the new files were created and syntactically inserted into the tree via local commit (static validation); no MATLAB runtime tests were executed in this environment because Octave/MATLAB was not available.
  • The new validation and profiling harnesses are provided to be run in the target MATLAB environment with real inputs and will perform the runtime equivalence and profiling checks (they are designed to fail closed if drift exceeds configured thresholds).

Codex Task

Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: c061ce4c93

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment on lines +13 to +17
[out95_rev9,outmc_rev9] = agg_check_parfor_chunk_rev9_app(app,agg_check_reliability,point_idx,sim_number,mc_size,radar_beamwidth,base_protection_pts,mc_percentile,on_list_bs,data_label1,reliability,norm_aas_zero_elevation_data,string_prop_model,single_search_dist,off_idx,min_azimuth,max_azimuth,custom_antenna_pattern,cell_aas_dist_data,cell_sim_data,sim_folder,parallel_flag);
runtime_rev9 = toc;

% Run rev10
tic;
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Separate rev9 and rev10 artifacts during validation

Running rev9 and rev10 back-to-back with the same arguments causes cross-contamination through shared checkpoint files: both implementations persist to the same *_array_agg_check_95_* and *_array_agg_check_mc_dBm_* names, so the rev10 call can short-circuit and reload rev9 outputs instead of executing its own compute path. In that case deep_compare can report a false PASS and the reported speedup is mostly measuring cache reuse, not rev10 behavior.

Useful? React with 👍 / 👎.

Comment on lines +18 to +20
wall_tic = tic;
[out95,outmc] = agg_check_parfor_chunk_rev9_app(app,agg_check_reliability,point_idx,sim_number,mc_size,radar_beamwidth,base_protection_pts,mc_percentile,on_list_bs,data_label1,reliability,norm_aas_zero_elevation_data,string_prop_model,single_search_dist,off_idx,min_azimuth,max_azimuth,custom_antenna_pattern,cell_aas_dist_data,cell_sim_data,sim_folder,parallel_flag); %#ok<NASGU>
wall_runtime_s = toc(wall_tic);
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Reset agg-check cache before profiling rev9

This profiler invocation does not clear or namespace rev9 checkpoint outputs before timing, but agg_check_parfor_chunk_rev9_app reuses persisted .mat results when they already exist. As a result, repeated profiling runs can measure the fast load path instead of the intended chunk/parfor compute path, which makes the top-contributor table and bucket percentages misleading for optimization decisions.

Useful? React with 👍 / 👎.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant