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

Tiogia TPL Update / Build Fixes #2485

Merged
merged 8 commits into from
Jun 4, 2023
Merged

Tiogia TPL Update / Build Fixes #2485

merged 8 commits into from
Jun 4, 2023

Conversation

wrtobin
Copy link
Collaborator

@wrtobin wrtobin commented May 25, 2023

@wrtobin wrtobin added type: feature New feature or request type: testing Unit tests, non-regression testing, ... type: build system Build system issue flag: requires updated submodule(s) labels May 25, 2023
@wrtobin wrtobin self-assigned this May 25, 2023
@wrtobin wrtobin changed the title Tioga Updates Tiogia TPL Update / Build Fixes May 25, 2023
@@ -35,7 +35,7 @@ void stringToInputVariable( Tensor< T, SIZE > & target, string const & inputValu
std::istringstream ss( inputValue );
auto const errorMsg = [&]( auto const & msg )
{
return GEOS_FMT( "{} for Tensor<{}> at position {} in input: {}", msg, SIZE, ss.tellg(), inputValue );
return GEOS_FMT( "{} for Tensor<{}> at position {} in input: {}", msg, SIZE, static_cast< int >( ss.tellg() ), inputValue );
Copy link
Collaborator Author

@wrtobin wrtobin May 25, 2023

Choose a reason for hiding this comment

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

Something about the tellg return type isn't handled appropriately by CCE on tioga using the more modern fmt libraray, but by the standard it is convertible to integer types, so this resolves the compile failure.

Copy link
Contributor

Choose a reason for hiding this comment

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

This is good fix for the future in general, since std::fpos is not required by the standard to have a std::formatter specialization

``ENABLE_DOCS`` ``ON`` Build documentation (Sphinx and Doxygen)
``ENABLE_WARNINGS_AS_ERRORS`` ``ON`` Treat all warnings as errors
``ENABLE_PVTPackage`` ``ON`` Enable PVTPackage library (required for compositional flow runs)
``ENABLE_TOTALVIEW_OUTPUT`` ``OFF`` Enables TotalView debugger custom view of GEOS data structures
``GEOS_ENABLE_TESTS`` ``ON`` Enables unit testing targets
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This was supposed to be in the unit test PR, but managed to be forgotten.

@wrtobin wrtobin marked this pull request as ready for review May 25, 2023 20:40
@bmhan12
Copy link
Contributor

bmhan12 commented May 25, 2023

tioga needs to be added here to "paths" so gitlab will save the created folder of artifacts for nightlyTests.

GEOS/.gitlab-ci.yml

Lines 91 to 98 in 461dadf

# Output a folder of logs files; these are automatically inherited by nightly-job
artifacts:
when: always
paths:
- quartz/
- lassen/
# Allow job to always fail, so nightly-job in next stage will run
allow_failure: true

Additionally, on the nightlyTests repository side of things, some scripting files need to modified so the tioga artifacts will be scraped and front-facing README updated:

Text file keeping track of when different phases were last passing. Entries need to be added for tioga, expected by the update.sh script.
https://github.com/GEOS-DEV/nightlyTests/blob/master/last_passed.txt

Add tioga to list of systems.
https://github.com/GEOS-DEV/nightlyTests/blob/3e8aadd1e6f9c522e5e1c67a6f628f45c4be2e40/update.sh#L19

@wrtobin
Copy link
Collaborator Author

wrtobin commented May 25, 2023

@bmhan12 Just opened a PR on nightlyTests with the noted changes: https://github.com/GEOS-DEV/nightlyTests/pull/7

@wrtobin
Copy link
Collaborator Author

wrtobin commented May 25, 2023

Would it break anything in the gitlab CI if altered the sha1 hashes in last_passed to 0000000 instead of just some copied hashes ( I assume we don't have any verification of those hashes, but you'd know better than I do ).

@bmhan12
Copy link
Contributor

bmhan12 commented May 25, 2023

Would it break anything in the gitlab CI if altered the sha1 hashes in last_passed to 0000000 instead of just some copied hashes ( I assume we don't have any verification of those hashes, but you'd know better than I do ).

Nope, no verification. In the past, I think I've put TBD or similar placeholder for when it has never passed before.

@wrtobin wrtobin added ci: run CUDA builds Allows to triggers (costly) CUDA jobs flag: requires updated TPL(s) Needs a specific TPL PR and removed flag: requires updated TPL(s) Needs a specific TPL PR labels May 25, 2023
@wrtobin
Copy link
Collaborator Author

wrtobin commented Jun 3, 2023

@CusiniM @rrsettgast

I was hoping to merge this just now but had a 3 hour timeout because of course. If I'm unable to merge it later on tonight and someone else gets to it first, once it goes in we can merge the nightlyTests PR linked above (it shouldn't go in first since it isn't a submodule and depends on this PR being merged to function correctly).

@CusiniM CusiniM merged commit b934da4 into develop Jun 4, 2023
@CusiniM CusiniM deleted the feature/wrtobin/tioga-ci branch June 4, 2023 23:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ci: run CUDA builds Allows to triggers (costly) CUDA jobs flag: requires updated submodule(s) type: build system Build system issue type: feature New feature or request type: testing Unit tests, non-regression testing, ...
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants