Skip to content

Better error reporting #2585

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.

Sign up for GitHub

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

Jump to bottom
Open
t-b wants to merge 9 commits into
base: main
Choose a base branch
from
Open

Better error reporting #2585

t-b wants to merge 9 commits into from

Conversation

@t-b
Copy link
Collaborator

@t-b t-b commented Nov 28, 2025 •
edited
Loading

Close #2568

@t-b t-b self-assigned this Nov 28, 2025
@t-b t-b force-pushed the bugfix/2585-better-error-reporting branch from a30e6fd to 31f83fc Compare December 1, 2025 20:32
@t-b t-b marked this pull request as ready for review December 1, 2025 21:18
Copilot AI review requested due to automatic review settings December 1, 2025 21:18
Copilot finished reviewing on behalf of t-b December 1, 2025 21:21
Copilot AI reviewed Dec 1, 2025
View reviewed changes
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR enhances error reporting capabilities in the MIES system by adding better runtime error detection, improving error messages, and including additional diagnostic information when assertions fail.

  • Added AssertOnAndClearRTError() calls before critical operations to prevent silent RTE suppression
  • Enhanced assertion output to include FIFO position and higher precision timestamps
  • Improved analysis function error reporting to distinguish between runtime errors and V_AbortCode aborts

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

File Description
Packages/MIES/MIES_Utilities_ProgramFlow.ipf Added AssertOnAndClearRTError() in ASSERT/ASSERT_TS functions, added FIFO position tracking to assertion output, and increased timestamp precision to 3 fractional seconds
Packages/MIES/MIES_MiesUtilities_Logbook.ipf Added AssertOnAndClearRTError() and AbortOnRTE to wave assignment operations, and added settingCol parameter to function call for optimization
Packages/MIES/MIES_ExperimentDocumentation.ipf Added AssertOnAndClearRTError() and AbortOnRTE to wave assignment loops to catch runtime errors early
Packages/MIES/MIES_AnalysisFunctionManagement.ipf Improved error reporting to distinguish between runtime errors and V_AbortCode, capturing error code in variable

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@t-b t-b assigned MichaelHuth and unassigned t-b Dec 1, 2025
MichaelHuth
MichaelHuth previously approved these changes Dec 2, 2025
View reviewed changes
Copy link
Collaborator

@MichaelHuth MichaelHuth left a comment

Choose a reason for hiding this comment

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

LGTM

@MichaelHuth MichaelHuth assigned t-b and unassigned MichaelHuth Dec 2, 2025
@t-b t-b force-pushed the bugfix/2585-better-error-reporting branch from d7d1200 to a4e2ad6 Compare December 2, 2025 18:53
@t-b

This comment was marked as outdated.

Sign in to view
@t-b t-b force-pushed the bugfix/2585-better-error-reporting branch from a4e2ad6 to 1fb5f1f Compare December 2, 2025 21:48
t-b added 9 commits December 3, 2025 21:43
@t-b
ASSERT/ASSERT_TS: Add AssertOnAndClearRTError in fail case
24d9c3f 
This should make finding the place of lingering RTEs easier.
@t-b
Catch lingering RTEs for labnotebook reading/writing
909bdb2 
This would have helped in debugging issue #2568.
@t-b
ASSERT/ASSERT_TS: Output time with milliseconds precision
f50f5e7
@t-b
ASSERT: Output fifo position
04f22c8
@t-b
GetLastSetting: Pass settingCol into GetLastSettingNoCache
beb1de0 
This then takes the fast path as we don't need to determine it again.
@t-b
AFM_CallAnalysisFunctions: Distinguish between RTEs and Aborts in cat…
baaa000 
…ch case

And also tone down the message do the user.
@t-b
MIES_IVSCC.ipf: Disable cache across new experiments [1]
3ecf2cd 
[1]: #2568 (comment)
@t-b
GetLBRowCache/GetLBIndexCache: Make them work across experiments
c6aa414 
Since 8be5388 (NewExperiment: Keep the cache waves across NewExperiment
calls, 2025-10-02) we allow to keep the cache across experiments.

For the labnotebook row and index cache keeping the cache across
experiments is equivalent to deleting the logbook waves and then
restarting over.

But the upgrade path for both cases did not work as expected as we only
did a full upgrade if the labnotebook was empty i.e. the GetLastSweepWithSetting
call did not succeed.

But if we added an entry for the first sweep, that would pass but then we
would not overwrite entries from later sweeps from the previous
labnotebook.
@t-b
IVS_NewExperimentWithCacheAndAutoloader: Enable cache across experime…
c35bc6b 
…nts again

This reverts commit 3ecf2cd (MIES_IVSCC.ipf: Disable cache across new experiments [1], 2025-12-02).

Fixed in the previous commit.
Copilot AI review requested due to automatic review settings December 3, 2025 21:36
@t-b t-b force-pushed the bugfix/2585-better-error-reporting branch from 8ab1bcf to c35bc6b Compare December 3, 2025 21:36
@t-b t-b assigned timjarsky and unassigned t-b Dec 3, 2025
Copilot finished reviewing on behalf of t-b December 3, 2025 21:39
Copilot AI reviewed Dec 3, 2025
View reviewed changes
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@t-b t-b requested a review from MichaelHuth December 3, 2025 21:46
@t-b t-b assigned MichaelHuth and t-b and unassigned timjarsky and MichaelHuth Dec 3, 2025
@t-b
Copy link
Collaborator Author

t-b commented Dec 4, 2025

  • Add and use the following function to invalidate all logbook (labnotebook, results) cache waves for all devices
/// @brief Invalidates the row and index cache for the given LBN
Function InvalidateLBIndexAndRowCache(WAVE values)
	CA_DeleteCacheEntry(CA_CreateLBIndexCacheKey(values))
	CA_DeleteCacheEntry(CA_CreateLBRowCacheKey(values))
End
  • Mention in GetLBRowCache/GetLBIndexCache that modCount is only tracking additions but that this is not a general purpose invalidation scheme.

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

Reviewers

Copilot code review Copilot Copilot left review comments

@timjarsky timjarsky Awaiting requested review from timjarsky timjarsky is a code owner

@MichaelHuth MichaelHuth Awaiting requested review from MichaelHuth MichaelHuth is a code owner

At least 1 approving review is required to merge this pull request.

Assignees

@t-b t-b

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

PSQ_DAScale: Asserts out with "Pulse duration can not be zero"

4 participants

@t-b @MichaelHuth @timjarsky