Added Transform template to SIe -- issues with movie tolog and fromlog functions to separate file, and with format;

[jbasabe]

PR Summary

PR Checklist

• 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.
• 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.
• Ensure that any when='@main' dependencies are updated to the release version in the package.py

[jhp-lanl]

Great work! That is a fantastic start!

[Yurlungur]

This is a great start!

[jhp-lanl]

Almost there I think! Provided that we're assuming transformed data already, I think there are minimal modifications needed to use the transformed data.

[jhp-lanl]

@Yurlungur now that I think more about it, I realize that we are likely assuming that the data is already transformed since we rely on sesame2spiner to actually determine the grid points where EOSPAC does interpolation.

If that's the case, then is it still okay for the transform functors to live in eos_spiner_rho_sie.hpp? We'll have to make sesame2spiner depend on this header then I think, right? Would it then make sense to move the transform classes into their own header?

[Yurlungur]

@Yurlungur now that I think more about it, I realize that we are likely assuming that the data is already transformed since we rely on sesame2spiner to actually determine the grid points where EOSPAC does interpolation.

Ah hmm... I think we need both right? Because in sesame2spiner we must generate a table on a square grid of (rho, sie - sie_cold) or (rho, transform(sie)). But then we need to go into the transformed space in spiner_eos_rho_sie when lookups are made.

If that's the case, then is it still okay for the transform functors to live in eos_spiner_rho_sie.hpp? We'll have to make sesame2spiner depend on this header then I think, right? Would it then make sense to move the transform classes into their own header?

This is a good point. I think the transform classes should move into their own header, yes.

[jbasabe]

@Yurlungur now that I think more about it, I realize that we are likely assuming that the data is already transformed since we rely on sesame2spiner to actually determine the grid points where EOSPAC does interpolation.

Ah hmm... I think we need both right? Because in sesame2spiner we must generate a table on a square grid of (rho, sie - sie_cold) or (rho, transform(sie)). But then we need to go into the transformed space in spiner_eos_rho_sie when lookups are made.

If that's the case, then is it still okay for the transform functors to live in eos_spiner_rho_sie.hpp? We'll have to make sesame2spiner depend on this header then I think, right? Would it then make sense to move the transform classes into their own header?

This is a good point. I think the transform classes should move into their own header, yes.

Ive separated the transformations into a separate header in this latest push. I've also still been working on the unit test for the transforms.

[jhp-lanl]

Ive separated the transformations into a separate header in this latest push. I've also still been working on the unit test for the transforms.

Looks good @jbasabe ! Let us know when you have the unit test up so we can review it.

[jbasabe]

Ive separated the transformations into a separate header in this latest push. I've also still been working on the unit test for the transforms.

Looks good @jbasabe ! Let us know when you have the unit test up so we can review it.
I'v e added a unit test for the Transfromations, as you can see it fails most of the GitHub test, im still not entirely familiar with how to debug those errors. Creating an appropriate databox for siecold gave me quite a bit of trouble as well and is probably not entirely what we want.

[jhp-lanl]

Just a few more small changes to the tests

[jhp-lanl]

@jbasabe other than the formatting, I think the tests are failing because of this:

/home/runner/work/singularity-eos/singularity-eos/test/test_spiner_transform.cpp:11:10: fatal error: 'spiner/databox.hpp' file not found
   11 | #include <spiner/databox.hpp>
      |          ^~~~~~~~~~~~~~~~~~~~
1 error generated.

I'm not quite sure why you're getting this error to be honest. @mauneyc-LANL or @Yurlungur do you have suggestions?

EDIT - oh wait.. I know. You need an #ifdef to check whether the spiner option is active. Since we're using databoxes directly, we need to make sure this test isn't run when we don't link to spiner.

[jbasabe]

@jbasabe other than the formatting, I think the tests are failing because of this:

/home/runner/work/singularity-eos/singularity-eos/test/test_spiner_transform.cpp:11:10: fatal error: 'spiner/databox.hpp' file not found
   11 | #include <spiner/databox.hpp>
      |          ^~~~~~~~~~~~~~~~~~~~
1 error generated.

I'm not quite sure why you're getting this error to be honest. @mauneyc-LANL or @Yurlungur do you have suggestions?

EDIT - oh wait.. I know. You need an #ifdef to check whether the spiner option is active. Since we're using databoxes directly, we need to make sure this test isn't run when we don't link to spiner.

I'm going to look into this more, but Im going to push again before our time is up, since most of the edits discussed I've trie to implement.

[jhp-lanl]

I think this looks good once the tests pass. I'll wait for @Yurlungur to also give his approval though in case I'm missing anything

[jbasabe]

I think this looks good once the tests pass. I'll wait for @Yurlungur to also give his approval though in case I'm missing anything

Just a fail on the formatting test, ive run the clang format script if thats the problem, but doesn't seem to change anything

[jhp-lanl]

@jbasabe let me see if I can fix the formatting issue...

[jhp-lanl]

@jbasabe I don't know why, but the script doesn't appear to be working correctly. Instead, I used something closer to what the CI is doing:

git ls-files -z '*.cpp' '*.hpp'   | xargs -0 clang-format-12 -style=file -i

and that seemed to work

[jhp-lanl]

The GPU tests caught some small things that need to be changed @jbasabe

[jbasabe]

The GPU tests caught some small things that need to be changed @jbasabe

I've made the edits and pushed to here and re-git