Skip to content

Conversation

@The9Cat
Copy link
Member

@The9Cat The9Cat commented Jul 31, 2025

The issue

The expMSSA header defines totVar and totPow as boolean but uses them as doubles to compute the norm. Not sure how this is legal code? But that's how it appears.

The fix

  1. Make totVar and totPow doubles.
  2. Add varFlag and powFlag as boolean class data set by the totVar and totPow YAM configuration flag
  3. Added both flags and values to the HDF5 cache files
  4. Added some verbose log output for testing to verify that the logic is working as expected.

Notes

While this is formally a big, I think, I'm basing the PR on devel, anticipating an upcoming merge to main

Copy link
Contributor

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 pull request fixes a bug in the expMSSA class where totVar and totPow were incorrectly declared as boolean types but used as double values for norm computations. The fix separates the control flags from the computed values and improves cache file handling.

  • Converts totVar and totPow from boolean to double types for norm calculations
  • Introduces new boolean flags varFlag and powFlag to control the normalization behavior
  • Adds cache file support for the new flags and improves HDF5 attribute handling

Reviewed Changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 5 comments.

File Description
include/SLGridMP2.H Adds static member variables for grid construction heuristics
exputil/SLGridMP2.cc Implements adaptive grid boundaries and enhanced diagnostic output
expui/expMSSA.cc Fixes boolean/double type confusion and adds flag handling
expui/expMSSA.H Updates class definition with separate flag and value variables

The9Cat and others added 4 commits July 31, 2025 18:32
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Copy link
Member

@michael-petersen michael-petersen left a comment

Choose a reason for hiding this comment

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

I can confirm this is all behaving as expected; still surprised the compiler didn't catch this, but seems that we are good now.

@michael-petersen michael-petersen merged commit b691df8 into devel Aug 16, 2025
8 checks passed
@michael-petersen michael-petersen deleted the mssaFlags branch August 16, 2025 08:46
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.

3 participants