Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

CI Overhaul #309

Merged
merged 52 commits into from
Nov 26, 2024
Merged

CI Overhaul #309

merged 52 commits into from
Nov 26, 2024

Conversation

ldowen
Copy link
Collaborator

@ldowen ldowen commented Oct 14, 2024

Summary

  • This PR overhauls the CI system.
  • It does the following:
    • Consolidates atstest.in, lcatstest.in, and run_ats.py scripts into a single spheral_ats.py script
    • Changed Gitlab CI from shell to batch
    • Improved how tests files are installed
    • Updated documentation regarding developer tools
    • CMake variable SPHERAL_TEST_INSTALL_PREFIX now includes the tests directory
    • Removed number of configured files, add SpheralConfigs.py.in file to compensate

ToDo :

  • Annotate RELEASE_NOTES.md with notable changes.
  • Create LLNLSpheral PR pointing at this branch. (PR#115)
  • LLNLSpheral PR has passed all tests.

…eation of test_list.yaml, add py-pyyaml as a new TPL to be able to read in the performance test list
…e inputs to adiak, updated testTimer to reflect these changes
…ingParser.py file instead of SpheralOptionParser.py, added ability to specify Adiak data directly from command line as a dictionary, update testTimer with adiakData tests and improved layout
…some new options for Noh tests, slight changes to performance.py
… files, changed SPHERAL_TEST_INSTALL_PREFIX to include tests directory, added tests for viz and restart files in SpheralController, created separate performance analysis python file
… parsing, removed use of None string in all existing tests, removed py-ats patch
@ldowen ldowen changed the title Feature/performance tests CI Overhaul Nov 13, 2024
@ldowen ldowen marked this pull request as ready for review November 13, 2024 22:14
.gitlab/scripts.yml Outdated Show resolved Hide resolved
.gitlab/scripts.yml Outdated Show resolved Hide resolved
scripts/CMakeLists.txt Show resolved Hide resolved
scripts/lc/install-from-dev-pkg.sh Outdated Show resolved Hide resolved
Comment on lines +105 to +123
def _get_arch(self):
host_platform = spack.platforms.host()
host_os = host_platform.operating_system("default_os")
host_target = host_platform.target("default_target")
architecture = spack.spec.ArchSpec((str(host_platform), str(host_os), str(host_target)))
spack_arch = str(architecture)
return spack_arch.strip()

# Create a name for the specific configuration being built
# This name is used to differentiate timings during performance testing
def _get_config_name(self, spec):
arch = self._get_arch()
config_name = f"{arch}_{spec.compiler.name}_{spec.compiler.version}"
if ("+mpi" in spec):
config_name += "_" + spec.format("{^mpi.name}_{^mpi.version}")
if ("+cuda" in spec):
config_name += "_" + spec.format("{^cuda.name}{^cuda.version}")
return config_name.replace(" ", "_")

Copy link
Collaborator

Choose a reason for hiding this comment

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

Why not do this in CMake?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Some of this relies on Spack internal methods for determining things. We would have to extract those methods into CMake otherwise.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I would prefer if we can do this during the CMake config stage at some point rather than relying on spack to generate a CMake string, but this is okay for now.

tests/CRKSPH.ats Show resolved Hide resolved
tests/performance.py Outdated Show resolved Hide resolved
…ml and install-from-dev-pkg, fixed recursive module bug from last commit
Copy link
Collaborator

@mdavis36 mdavis36 left a comment

Choose a reason for hiding this comment

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

Just a change in install-from-dev-pkgand I think this will be good

Copy link
Collaborator

@mdavis36 mdavis36 left a comment

Choose a reason for hiding this comment

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

Good to go

@ldowen ldowen merged commit e282594 into develop Nov 26, 2024
2 checks passed
@ldowen ldowen deleted the feature/performance_tests branch November 26, 2024 23:30
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