1) 3 Tests for testing PTESolverRhoT versus Ann's Flag PTsolv 2) Implementing Carl Grief's mass fraction update model.

[aematts]

PR Summary

1. Created and added 3 tests for making sure PTESolverRhoT gives the same as Ann's Flag PTsolv.
2. Implementing a skeleton scheme for introducing a Kinetic PhaseTransition scheme into host codes.
   In the first round we only implement ingredients one by one and assemble them in test cases. Later we might
   introduce classes, move subroutines around for better structure, and add more advanced C++ features (for example Indexers) to make future expansions/variations easier to handle. The division into host and singularity-eos code is floating for the moment.

PR Checklist

• [ x] 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.
• [ x] 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:

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

[aematts]

@jhp-lanl and @Yurlungur here is the WIP PR that I promised. This is a real WIP since I have several issues that I need help with. One issue is: the 2phase cpp file can use two header files pte_longtest_2phase...... and pte_test_2phase...
Is it possible to make two test cases out of this without duplicating the cpp file?
Then the tests seem to run using ctest -R name -v on Darwin (skylake-gold) but at least one of the tests here (Minimal) has failed (so far).

[jhp-lanl]

Thanks for putting this up @aematts and thanks for digging into what isn't necessarily intuitive C++ in a lot of this!

@jhp-lanl and @Yurlungur here is the WIP PR that I promised. This is a real WIP since I have several issues that I need help with. One issue is: the 2phase cpp file can use two header files pte_longtest_2phase...... and pte_test_2phase... Is it possible to make two test cases out of this without duplicating the cpp file? Then the tests seem to run using ctest -R name -v on Darwin (skylake-gold) but at least one of the tests here (Minimal) has failed (so far).

If I understand your question, I think the answer is namespaces. It looks like you use a lot of the same names in both the headers so an easy way to differentiate between the two would be to wrap the body of each header in a namespace.

So something like this in the header

namespace longtest_2phaseVinetSn {
constexpr int NMAT = 2;
constexpr int NTRIAL = 20;
constexpr int NPTS = NTRIAL * NMAT;
...
} // namespace longtest_2phaseVinetSn

And then when you want to use it in the actual test, you could do something like this:

longtest_2phaseVinetSn::set_eos(eos_hv.data());

I didn't take the time to track down what exactly you want from the header files, but this was the first example I could find where it might be different.

I think there is a lot of duplicate code between the headers as well that you may want to break out into a common header that they can all use. For example are the indexers all different? They didn't look different but it's possible I missed something.

[aematts]

Thanks for putting this up @aematts and thanks for digging into what isn't necessarily intuitive C++ in a lot of this!

@jhp-lanl and @Yurlungur here is the WIP PR that I promised. This is a real WIP since I have several issues that I need help with. One issue is: the 2phase cpp file can use two header files pte_longtest_2phase...... and pte_test_2phase... Is it possible to make two test cases out of this without duplicating the cpp file? Then the tests seem to run using ctest -R name -v on Darwin (skylake-gold) but at least one of the tests here (Minimal) has failed (so far).

If I understand your question, I think the answer is namespaces. It looks like you use a lot of the same names in both the headers so an easy way to differentiate between the two would be to wrap the body of each header in a namespace.

So something like this in the header

namespace longtest_2phaseVinetSn {
constexpr int NMAT = 2;
constexpr int NTRIAL = 20;
constexpr int NPTS = NTRIAL * NMAT;
...
} // namespace longtest_2phaseVinetSn

And then when you want to use it in the actual test, you could do something like this:

longtest_2phaseVinetSn::set_eos(eos_hv.data());

I didn't take the time to track down what exactly you want from the header files, but this was the first example I could find where it might be different.

I think there is a lot of duplicate code between the headers as well that you may want to break out into a common header that they can all use. For example are the indexers all different? They didn't look different but it's possible I missed something.

Good idea with one common header file for the things that appear in both header files. It is only the data sets that differ between the header files.
But the namespace solution that you suggests for the two header files is not what I am looking for.

What I am looking for is for the compiler to create two tests: 1) one with the test header file and test_pte_2phase.cpp and 2) one with the longtest header file and test_pte_2phase.cpp.

I will check on what failed in the checks.

[jhp-lanl]

What I am looking for is for the compiler to create two tests: 1) one with the test header file and test_pte_2phase.cpp and 2) one with the longtest header file and test_pte_2phase.cpp.

Ahhhhh sorry I misunderstood.

Really the best solution is to not mirror the existing PTE test since it's not really a well-written test and just use the Catch test framework we use for all the other tests. This way, you can define things common to both tests at a higher level in the test (see the Gruneisen test you wrote as an example) and then have lower levels specify the things that are different.

One solution with minimal code changes would be to change each test from having a main() function without arguments to some other function that takes the arguments you want to change in each test. Then this could also become a header and then the .cpp file would just wrap the function in a main() and call with the different parameter sets, maybe propagating the return value so that main() will return zero only if both tests pass.

[Yurlungur]

Thanks for adding these @aematts !

So if I understand your question above---the issue is that test_pte_*phase.cpp is identical for both CPP files just with different headers that define set_eos is that right? And the function signature of set_eos is the same and `Indexer2D is the same between all the files?

I think the first step to cleaning that up a bit is to define a void function that does all the stuff in side the test cpp file between Kokkos::initialize and Kokkos::finalize. It would have a signature of something like this:

template<typename SetEos_t>
void TestPTE(const SetEos_t &set_eos) {
  /* all the stuff thats currently in there *./
}

Then you can call this function twice in the same cpp file with the two different set_eos functions defined in your headers, so long as they're named slightly differently. You can name one set_eos_2pte and one set_eos_3pte, or use namespaces as @jhp-lanl suggested. So that would look something like this:

test_pte(2phase::set_eos);

test_pte(3phase::set_eos);

From there, we can think about adding the catch2 stuff. But I think this gets you most of what you want. Does that seem like a good path?

[Yurlungur]

I think these two if statements can be combined. :)

Suggested change

	endif()
	if(SINGULARITY_USE_SPINER)

[Yurlungur]

Same as above... a comment about where these came from would be nice.

[aematts]

I just pushed my first cleanup attempt. My plan is to write one .cpp for all pte tests. But I have not yet fixed test_pte_{2,3}phase.cpp (and hopefully even test_pte.cpp) to be the same, I have to think a little more about it. But in the end I want to use one .hpp file (or rather namespace) for each test and line the tests up in one .cpp file like the other tests, using a common testPTE function.

[aematts]

To do next: Fix text output to be outside of what will be testPTE, and use same input format (mainly trial_vfrac[NMAT][NTRIAL] and in_lambda[NMAT][NTRIAL]) for all calls to testPTE. Then it should be ready for breaking out testPTE.

[aematts]

@Yurlungur , would you mind if I put test_pte into this new framework? That means changing set_state to not have t as input parameter but instead n, the trial number. But it should be the same, just setting t=600.0 as an input in the namespace and get sie from it in in the .hpp file.

[aematts]

I will fix the formatting and add a comment on where the data is from before I leave this for a week or so. I will also try to figure out why I get this now:
/home/runner/work/singularity-eos/singularity-eos/test/pte_test_5phaseSesameSn.hpp:69:42: error: ‘EOSPAC’ is not a member of ‘singularity’
69 | singularity::EOS Snbeta = singularity::EOSPAC(SnbetaID);
| ^~~~~~

[Yurlungur]

@Yurlungur , would you mind if I put test_pte into this new framework? That means changing set_state to not have t as input parameter but instead n, the trial number. But it should be the same, just setting t=600.0 as an input in the namespace and get sie from it in in the .hpp file.

I wouldn't mind at all. I think that would be a nice cleanup!

[jhp-lanl]

I will fix the formatting and add a comment on where the data is from before I leave this for a week or so. I will also try to figure out why I get this now: /home/runner/work/singularity-eos/singularity-eos/test/pte_test_5phaseSesameSn.hpp:69:42: error: ‘EOSPAC’ is not a member of ‘singularity’ 69 | singularity::EOS Snbeta = singularity::EOSPAC(SnbetaID); | ^~~~~~

In case you haven't fixed this, this to me seems like maybe EOSPAC isn't being slurped in for the testing either because it's not being detected in the environment or because it's not enabled in the build options.

[aematts]

I fixed it. Thanks. Ann From: Jeff Peterson ***@***.***> Reply-To: lanl/singularity-eos ***@***.***> Date: Thursday, May 2, 2024 at 6:43 PM To: lanl/singularity-eos ***@***.***> Cc: Ann Wills ***@***.***>, Mention ***@***.***> Subject: [EXTERNAL] Re: [lanl/singularity-eos] WIP: 3 Tests for testing PTESolverRhoT versus Ann's Flag PTsolv (PR #361) I will fix the formatting and add a comment on where the data is from before I leave this for a week or so. I will also try to figure out why I get this now: /home/runner/work/singularity-eos/singularity-eos/test/pte_test_5phaseSesameSn.hpp:69:42: error: ‘EOSPAC’ is not a member of ‘singularity’ 69 | singularity::EOS Snbeta = singularity::EOSPAC(SnbetaID); | ^~~~~~ In case you haven't fixed this, this to me seems like maybe EOSPAC isn't being slurped in for the testing either because it's not being detected in the environment or because it's not enabled in the build options. — Reply to this email directly, view it on GitHub<https://urldefense.com/v3/__https:/github.com/lanl/singularity-eos/pull/361*issuecomment-2091961980__;Iw!!Bt8fGhp8LhKGRg!GghNieaBkKrFhrJ5Tks8As0v465pkx3k2wew46iKpo89jY-l7Mfptaze_UsoTN8Jm807gWcepQOM2UtwY5rKpwKI$>, or unsubscribe<https://urldefense.com/v3/__https:/github.com/notifications/unsubscribe-auth/ALGV2P6R5EK2LTTMFK6KNO3ZALMULAVCNFSM6AAAAABGKLTYLGVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDAOJRHE3DCOJYGA__;!!Bt8fGhp8LhKGRg!GghNieaBkKrFhrJ5Tks8As0v465pkx3k2wew46iKpo89jY-l7Mfptaze_UsoTN8Jm807gWcepQOM2UtwY_4aymIM$>. You are receiving this because you were mentioned.Message ID: ***@***.***>

[aematts]

the re-git pipeline include the PTE solver tests but not the KPT tests:
Start 53: Scenario: Density-Temperature PTE Solver
53/67 Test #53: Scenario: Density-Temperature PTE Solver ........................................... Passed 0.47 sec
Start 54: pte
54/67 Test #54: pte ................................................................................ Passed 0.04 sec
Start 55: pte_2phase
55/67 Test #55: pte_2phase ......................................................................... Passed 0.03 sec
Start 56: pte_3phase
56/67 Test #56: pte_3phase ......................................................................... Passed 1.3

[aematts]

Hm, Jeff wanted me to move the KPT tests down to the closure section in CMakeLists, that is probably why.

Please help me figure this out. Where should we test what.

I also use EOSPAC for one pte test and that doesn't show up on GitHub either.

[aematts]

Same result as before on re-git. But my kpt tests are not in there. But the GPU's choke on other things anyway, some of my pte tests but also eos_gruneisen and eos_variant. So it is not only my fault.

[Yurlungur]

Same result as before on re-git. But my kpt tests are not in there. But the GPU's choke on other things anyway, some of my pte tests but also eos_gruneisen and eos_variant. So it is not only my fault.

The automated testing is currently broken on re-git on GPUs. @rbberger is fixing it.

We should be able to manually test on CPU and GPU on Darwin though. The PTE tests only run on the yellow, not on github because we don't have access to the sesame database on the open. Let me give it a try on your branch.

[aematts]

I am testing it with
cmake .. --preset="basic_with_testing" -DSINGULARITY_SUBMODULE_MODE=ON -DSINGULARITY_BUILD_CLOSURE=ON

on skylake-gold just now. On Darwin.

I am not sure this includes SPINER but anyway.

[Yurlungur]

cmake .. --preset="basic_with_testing" -DSINGULARITY_SUBMODULE_MODE=ON -DSINGULARITY_BUILD_CLOSURE=ON

👍 I'll try with CPU and GPU and see if I can figure out what's going on with running/not running your tests. stay tuned.

[aematts]

I am testing it with cmake .. --preset="basic_with_testing" -DSINGULARITY_SUBMODULE_MODE=ON -DSINGULARITY_BUILD_CLOSURE=ON

on skylake-gold just now. On Darwin.

I am not sure this includes SPINER but anyway.

No, my KPT tests did not run :(.

Will continue looking at it.

[aematts]

When I comment out #ifdef PORTABILITY_STRATEGY_NONE it works:
Start 54: Scenario: First log rate test
54/68 Test #54: Scenario: First log rate test ...................................................... Passed 0.01 sec
Start 55: pte
55/68 Test #55: pte ................................................................................ Passed 0.01 sec
Start 56: pte_2phase
56/68 Test #56: pte_2phase ......................................................................... Passed 0.01 sec
Start 57: pte_3phase
57/68 Test #57: pte_3phase ......................................................................... Passed 0.25 sec

[aematts]

The current version now runs on GitHub and on the CPU part of re-git. And it doesn't run on the GPU part.
I think that is enough for now.

re-git GPU is complaining about std::cout stuff in my 2phase and 3phase pte tests. Should I put in the #ifndef PORTABILITY_STRATEGY_KOKKOS there too? Or can that wait for now (there are complaints about other tests as well so it will still not pass).

If so I am done and you can merge.

[Yurlungur]

When I comment out #ifdef PORTABILITY_STRATEGY_NONE it works: Start 54: Scenario: First log rate test 54/68 Test #54: Scenario: First log rate test ...................................................... Passed 0.01 sec Start 55: pte 55/68 Test #55: pte ................................................................................ Passed 0.01 sec Start 56: pte_2phase 56/68 Test #56: pte_2phase ......................................................................... Passed 0.01 sec Start 57: pte_3phase 57/68 Test #57: pte_3phase ......................................................................... Passed 0.25 sec

@aematts I found the problem. PORTABILITY_STRATEGY_NONE is only for builds without Kokkos. But there are CPU builds with Kokkos. That means my original idea for a solution isn't so simple as I thought. That's on me. Let me see if I can get your code building on GPUs. Then I'd like @rbberger 's CI MR to go through (it's up now and should be merged soon.) Then I'll click merge on yours. Thanks for all the hard work on this!

[Yurlungur]

There were a few minor issues, now resolved:

1. Some minor things with GPU builds and, e.g., cout vs printf.
2. EOSPAC doesn't currently build in a way that it can be called on device with the closure tooling so I disabled the 3phase test for device builds. The test compiles but doesn't do anything.
3. GPU builds were very slow because the variant was being rebuilt in each test. I know choose m minimal variant. Compile times are much shorter.

[Yurlungur]

Will now merge as soon as CI passes on re-git and github.

[Yurlungur]

@aematts I've hit a small snag on the CI. The material IDs used for the 5phase sesame test are in a sesame data base curated by you. This doesn't look like it's available on all CI machines, such as Rocinante. I'm looping in @rbberger . Let's continue the conversation by email.

[Yurlungur]

Tests now passing on re-git. Will merge as soon as tests pass on github.

[jhp-lanl]

Thanks for shepherding this to completion @Yurlungur !