Add introspection to `get_sg_eos` interface #382

jhp-lanl · 2024-06-26T02:18:24Z

PR Summary

When the library is built with debug flags, this adds introspection functions to the init and final functors in the get_sg_eos interface.

These functions check to make sure the values being passed are either zero or normal. I'm using slightly more complicated logic than just is_finite so that I can detect underflow values as well. For some quantities, I'm also including checks to make sure the returned value is strictly positive or non-negative where appropriate.

PR Checklist

N/A Adds a test for any bugs fixed. Adds tests for new features.
Format your changes by using the make format command after configuring with cmake.
N/A Document any new features, update documentation for changes made.
Make sure the copyright notice on any files you modified is up to date.
After creating a pull request, note it in the CHANGELOG.md file.
LANL employees: make sure tests pass both on the github CI and on the Darwin CI

If preparing for a new release, in addition please check the following:

N/A Update the version in cmake.
N/A Move the changes in the CHANGELOG.md file under a new header for the new release, and reset the categories.

jhp-lanl · 2024-06-26T02:31:42Z

@Yurlungur @dholladay00 is going to be a bit unavailable for the next few days so I might need your review here.

Also please check me on whether you agree with what I'm requiring to be non-negative or strictly positive.

jhp-lanl · 2024-06-26T02:57:39Z

FYI - I did some reading on and vs && and it seems like the consensus is that because && has been used traditionally, it is preferred over and. Coming from Fortran and python, I find and more readable, but I'm happy to change them to the more commonly-used C++ symbols.

Yurlungur · 2024-06-26T14:53:00Z

FYI - I did some reading on and vs && and it seems like the consensus is that because && has been used traditionally, it is preferred over and. Coming from Fortran and python, I find and more readable, but I'm happy to change them to the more commonly-used C++ symbols.

I strongly prefer &&. I wasn't even aware and was valid C++.

jhp-lanl · 2024-06-26T16:48:26Z

FYI - I did some reading on and vs && and it seems like the consensus is that because && has been used traditionally, it is preferred over and. Coming from Fortran and python, I find and more readable, but I'm happy to change them to the more commonly-used C++ symbols.

I strongly prefer &&. I wasn't even aware and was valid C++.

Apparently it's a holdover from the ancient times when some keyboards had limited symbols believe it or not. Again, I just liked it because coming from a non-C++ background it seems more readable. I'll change it

…equipartitioning

singularity-eos/eos/get_sg_eos_functors.hpp

mauneyc-LANL

Approve, with comments.

singularity-eos/eos/get_sg_eos_functors.hpp

Yurlungur

Overall this is a nice (and necessary) improvement. I have some nitpicks below, though.

singularity-eos/eos/get_sg_eos_functors.hpp

…ation

…cutoff for PTE participation

…nstructor

jhp-lanl · 2024-06-27T02:31:31Z

@jonahm-LANL or @mauneyc-LANL any ideas on what I'm doing wrong here?

/home/runner/work/singularity-eos/singularity-eos/singularity-eos/eos/singularity_eos.f90:410: undefined reference to `get_sg_eos'

I added the mass_frac_cutoff argument to the get_sg_eos() interface function both on the C++ side and the fortran side, and I'm adding an optional argument to the fortran interface. I can't for the life of me see where my mistake is and why the linker wouldn't be able to see get_sg_eos(). Any help is appreciated!

jhp-lanl · 2024-06-27T02:49:24Z

@jonahm-LANL or @mauneyc-LANL any ideas on what I'm doing wrong here?
/home/runner/work/singularity-eos/singularity-eos/singularity-eos/eos/singularity_eos.f90:410: undefined reference to `get_sg_eos'
I added the mass_frac_cutoff argument to the get_sg_eos() interface function both on the C++ side and the fortran side, and I'm adding an optional argument to the fortran interface. I can't for the life of me see where my mistake is and why the linker wouldn't be able to see get_sg_eos(). Any help is appreciated!

@dholladay00 with the fix. I forgot that the get_sg_eos() prototype exists in the singularity_eos.hpp header even though the function itself is defined the get_sg_eos.cpp file.

jhp-lanl · 2024-06-27T17:01:24Z

utils/scripts/format.sh

@@ -39,6 +39,6 @@ for f in $(git grep --untracked -ail res -- :/*.hpp :/*.cpp); do
    if [ ${VERBOSE} -ge 1 ]; then
       echo ${f}
    fi
-    ${CFM} -i ${REPO}/${f}
+    ${CFM} -style=file -i ${REPO}/${f}


FYI - this change just brings the format script in line with what the CI is doing. I don't think its omission was really messing anything up, but I just made the change as part of trying to figure out why my clang format wasn't working.

jhp-lanl · 2024-06-27T17:07:16Z

@mauneyc-LANL and @Yurlungur let me know if the most recent changes have addressed your comments. I think the new error_utils.hpp file needs a fresh review

Yurlungur

I'm happy now. Just one question below, but it is non-blocking.

singularity-eos/base/error_utils.hpp

jhp-lanl · 2024-06-28T01:12:31Z

I'm having trouble with one machine on our gitlab CI (network cloning issues... possibly proxy problems). Can one of you run the CI for me there? I think this is good to go otherwise.

Yurlungur · 2024-06-28T15:46:15Z

I'm having trouble with one machine on our gitlab CI (network cloning issues... possibly proxy problems). Can one of you run the CI for me there? I think this is good to go otherwise.

Looks like it's passing on gitlab. Going to pull the trigger.

jhp-lanl added 4 commits June 25, 2024 19:43

Add introspection functions to functors to locate bad values

5678e51

Provide PTE success/failure to final functor

05377f4

Add checks for negative and zero values (where appropriate)

e042a6a

Clang format

4626454

jhp-lanl requested review from Yurlungur and dholladay00 June 26, 2024 02:18

jhp-lanl added Robustness Ensures that existing features work as intended interface Library interface work labels Jun 26, 2024

jhp-lanl added 3 commits June 25, 2024 20:20

Update changelog

1d66ace

Slightly change output formatting

b5187bc

update copyright

d285b13

jhp-lanl requested a review from mauneyc-LANL June 26, 2024 02:32

jhp-lanl added 4 commits June 26, 2024 10:59

Change 'and' to '&&'

9b69e76

Change 'or' to '||'

e90bf8c

Change volume fraction guess to be equal to mass fraction instead of …

91b0544

…equipartitioning

Typo

04e9820

jhp-lanl commented Jun 26, 2024

View reviewed changes

singularity-eos/eos/get_sg_eos_functors.hpp Show resolved Hide resolved

mauneyc-LANL approved these changes Jun 26, 2024

View reviewed changes

Yurlungur requested changes Jun 26, 2024

View reviewed changes

jhp-lanl added 7 commits June 26, 2024 19:03

Add error utilities for checking conditions and printing error inform…

db35049

…ation

Add optional argument to get_sg_eos_f() to specify the mass fraction …

996db13

…cutoff for PTE participation

Use error_utils.hpp functions and add mass fraction cutoff to init co…

2f10112

…nstructor

Clang format

23e8e24

Make index variables auto type

ca97060

Fix indentation for clang format

5cf0dac

my clang format is broken?

880a803

jhp-lanl added 3 commits June 26, 2024 20:07

Add format config file to format script

ad55c7a

Maybe finally fix clang format?

9a95268

Change to appropriate Fortran input type for get_sg_eos_f()

08d7828

jhp-lanl added 2 commits June 26, 2024 20:37

Move argument for better readability

32398d9

Change get_sg_eos function prototype to match function

2f78b47

jhp-lanl commented Jun 27, 2024

View reviewed changes

jhp-lanl added 6 commits June 27, 2024 11:15

Update changelog to reflect new mass frac cutoff

02ddf39

Adjust formatting and add copyright

b9000ff

Update copyright

0b0940b

Add units to state output

174029f

Add spvol to final functor

97bc6dd

Clang format

f0e2d60

jhp-lanl mentioned this pull request Jun 27, 2024

Fix Bug in PTE Solver and Several Code Quality Improvements #383

Merged

6 tasks

jhp-lanl added 2 commits June 27, 2024 13:31

Add missing initializer

503750f

Clang format

8237b71

Yurlungur approved these changes Jun 27, 2024

View reviewed changes

singularity-eos/base/error_utils.hpp Outdated Show resolved Hide resolved

jhp-lanl added 2 commits June 27, 2024 17:12

Revert to C++14 compatbile type trait

cc1721f

Correct heat capacity units

033e6ee

Yurlungur merged commit dcd7a6e into main Jun 28, 2024
5 checks passed

Yurlungur deleted the jhp/get_sg_introspection branch June 28, 2024 15:46

jhp-lanl mentioned this pull request Jul 11, 2024

Create generic PTE state check #388

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add introspection to `get_sg_eos` interface #382

Add introspection to `get_sg_eos` interface #382

jhp-lanl commented Jun 26, 2024 •

edited

Loading

jhp-lanl commented Jun 26, 2024

jhp-lanl commented Jun 26, 2024

Yurlungur commented Jun 26, 2024

jhp-lanl commented Jun 26, 2024

mauneyc-LANL left a comment

Yurlungur left a comment

jhp-lanl commented Jun 27, 2024

jhp-lanl commented Jun 27, 2024

jhp-lanl Jun 27, 2024

jhp-lanl commented Jun 27, 2024

Yurlungur left a comment

jhp-lanl commented Jun 28, 2024

Yurlungur commented Jun 28, 2024

Add introspection to get_sg_eos interface #382

Add introspection to get_sg_eos interface #382

Conversation

jhp-lanl commented Jun 26, 2024 • edited Loading

PR Summary

PR Checklist

jhp-lanl commented Jun 26, 2024

jhp-lanl commented Jun 26, 2024

Yurlungur commented Jun 26, 2024

jhp-lanl commented Jun 26, 2024

mauneyc-LANL left a comment

Choose a reason for hiding this comment

Yurlungur left a comment

Choose a reason for hiding this comment

jhp-lanl commented Jun 27, 2024

jhp-lanl commented Jun 27, 2024

jhp-lanl Jun 27, 2024

Choose a reason for hiding this comment

jhp-lanl commented Jun 27, 2024

Yurlungur left a comment

Choose a reason for hiding this comment

jhp-lanl commented Jun 28, 2024

Yurlungur commented Jun 28, 2024

Add introspection to `get_sg_eos` interface #382

Add introspection to `get_sg_eos` interface #382

jhp-lanl commented Jun 26, 2024 •

edited

Loading