Skip to content

Fix aggregate_vector_diagnosis description indent handling#147

Merged
bonachea merged 1 commit intoBerkeleyLab:mainfrom
bonachea:fix-aggregate_vector_diagnosis
Dec 10, 2025
Merged

Fix aggregate_vector_diagnosis description indent handling#147
bonachea merged 1 commit intoBerkeleyLab:mainfrom
bonachea:fix-aggregate_vector_diagnosis

Conversation

@bonachea
Copy link
Copy Markdown
Member

@bonachea bonachea commented Dec 9, 2025

Fix a bug in aggregate_vector_diagnosis that breaks Julienne diagnostic string aggregation with .also..

Currently given an incremental diagnosis pattern like this:

diag = .true.
diag = .diag. .also. .false. // "whatever"
do i=1,100
  diag = .diag. .also. .true.
end do

During the loop the code in aggregate_vector_diagnosis will prepend 100 new_line_indent strings into the diagnosis string (ie 100 blank lines), even though there was only one actual failure.

Fundamentally the problem is that a new_line_indent is being prepended each time aggregate_vector_diagnosis is called with a failing argument, even if the failing diagnosis was the outcome of a previous call to aggregate_vector_diagnosis that already prepended a new line.

The surgical solution in this PR prevents this defect. As a side-benefit it also provides nicer behavior if the client has formatted their own output line and already inserted a line break, eg:

diag = test_diagnosis_t(.false.,new_line('') // "  I formatted this line myself!")

@bonachea bonachea requested a review from rouson December 9, 2025 23:51
Copy link
Copy Markdown
Contributor

@rouson rouson left a comment

Choose a reason for hiding this comment

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

LGTM

Fix a bug in aggregate_vector_diagnosis that breaks Julienne diagnostic string agreggation with .also.
Currently given an incremental diagnosis pattern like this:
```fortran
diag = .true.
diag = .diag. .also. .false. // "whatever"
do i=1,100
  diag = .diag. .also. .true.
end do
```
The code in aggregate_vector_diagnosis will prepend 100 `new_line_indent` strings into the diagnosis string (ie 100 blank lines), even though there was only one actual failure.

Fundamentally the problem is that a `new_line_indent` is being prepended each time `aggregate_vector_diagnosis` is called with a failing argument, even if the failing diagnosis was the outcome of a previous call to `aggregate_vector_diagnosis` that already prepended a new line.

The surgical solution in this PR prevents this defect. As a side-benefit it also provides nicer behavior if the client has formatted their own output line and already inserted a line break, eg:
```fortran
diag = test_diagnosis_t(.false.,new_line('') // "  I formatted this line myself!")
```
@bonachea bonachea force-pushed the fix-aggregate_vector_diagnosis branch from d40af91 to b8fc996 Compare December 10, 2025 19:03
@bonachea bonachea merged commit 0f3d987 into BerkeleyLab:main Dec 10, 2025
27 checks passed
@bonachea bonachea deleted the fix-aggregate_vector_diagnosis branch December 10, 2025 20:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants