Skip to content

fix: renamed preprocessor conditionals for SLOW_LIGHT#111

Merged
gnwong merged 1 commit intoAFD-Illinois:masterfrom
pedronaethe:master
Apr 30, 2026
Merged

fix: renamed preprocessor conditionals for SLOW_LIGHT#111
gnwong merged 1 commit intoAFD-Illinois:masterfrom
pedronaethe:master

Conversation

@pedronaethe
Copy link
Copy Markdown
Contributor

@pedronaethe pedronaethe commented Apr 30, 2026

This PR fixes a major bug in slowlight where the function interp_scalar_time was executed wrongly due to incorrect preprocessor conditional directives.

As a result, plasma variables were not interpolated across dump files. Instead, the code returned values from the earliest loaded dump between the two that should be interpolated, leading to incorrect results without triggering any warnings or runtime errors.

The issue has been corrected by fixing the conditional compilation directive controlling slowlight. I have verified that it compiles successfully, but haven't tested runtime behavior.

@pedronaethe pedronaethe changed the title MAJOR BUG FIX: renamed preprocessor conditionals for SLOW_LIGHT fix: renamed preprocessor conditionals for SLOW_LIGHT Apr 30, 2026
@gnwong
Copy link
Copy Markdown
Collaborator

gnwong commented Apr 30, 2026

Wow, this is quite concerning. I think this actually does not affect (most of?) the slow light runs that we generated for other projects (looking at my version of the slow light code), but it should definitely be fixed here. It's also not completely clear to me which runs are affected (I'm sure some of the runs I'd generated are), so perhaps the safest advisement is to redo anything that someone can't trace the progeny of. That said, I know we typically use high cadence GRMHD snapshots and test convergence in snapshot sampling rate, so hopefully the effects of this aren't too severe...

@gnwong
Copy link
Copy Markdown
Collaborator

gnwong commented Apr 30, 2026

@c-prather @avjoshi21 @vedantdhruv96 @cfgammie I'm accepting this and merging it now but figured I should tag you all just in case you didn't see.

@gnwong gnwong merged commit 7f7a482 into AFD-Illinois:master Apr 30, 2026
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