search

openshmem-org / tests-sos

Sandia OpenSHMEM unit tests and performance testing suite
Other
6 stars 11 forks source link

Sync with SOS v1.5.0 #16

Closed davidozog closed 3 years ago

davidozog commented 3 years ago

This incorporates all changes that are soon to be released in SOS v1.5.0. It's a substantial amount of shmemx->shmem renaming and reorganization.

Everything has been reviewed and tested on the SOS side (mostly here: https://github.com/Sandia-OpenSHMEM/SOS/pull/955), but this PR could certainly use a sanity check. Can I get a volunteer to take a look and give it the go-ahead? (perhaps @minsii, @jdinan, or @wrrobin)

@minsii, are you still a user of these tests? If so, please note all the shmemx updates.

One pending issue @jdinan noticed is https://github.com/Sandia-OpenSHMEM/SOS/issues/977, that some spec-examples won't build with a c99-only compiler. We should be able to disable these tests in SOS when c11 isn't supported, but such a fix would need to be merged with tests-sos/configure.ac. I'll look into this asap, but I may prefer to defer fixing that issue until v1.5.1, unless anybody objects?

minsii commented 3 years ago

@davidozog Yes we are still using sos test suite for OSHMPI tests. We are using an old version at https://github.com/minsii/tests-sos in order to avoid unnecessary fixes due to API changes. The 1.5 APIs development in OSHMPI is still WIP. Once it is in, we will pull the sos test suite upstream.

davidozog commented 3 years ago

Thanks @minsii - I figured that might be the case. I wonder if the v1.4.5 tag on this repo will suite OSHMPI, or if there's something custom on your fork you need?

So it sounds like you're ok with merging this?

The issue I mentioned (SOS issue #977) has a possible fix posted (SOS PR #986), so we may merge that in here as well depending on what Wasi says about it (after vacation) ;)

minsii commented 3 years ago

@davidozog Let me check v1.4.5, will get back to you shortly.

minsii commented 3 years ago

@davidozog we have two additional commits which are not included in v1.4.5 (see https://github.com/minsii/tests-sos/tree/env-trials)

We keep[dc050] as our own customization for now. Do you think it can be added into tests-sos master?

davidozog commented 3 years ago

@minsii Thanks for looking into that. As you probably know, we prefer to keep tests-sos and SOS as synced as possible. I could potentially prepare a v1.4.x branch where we can include e3ca4 (and dc050, as well as future changes you may need). That way you could point OSHMPI's testing to v1.4.x of tests-sos until v1.5 support is ready. Let me know if you'd like this, and I'll prepare that (we have an SOS v1.3.x branch as well that was apparently never needed in tests-sos).

And I don't see any problem adding dc050 to master on both tests-sos and SOS (except sometimes the Coverity scanner is leery of getenv, but this should be fine). Do you want to submit the pull request?

minsii commented 3 years ago

Thanks for your support @davidozog. The 1.4.x plan sounds great to me. Sure, will submit the PR shortly.

minsii commented 3 years ago

See #17

jdinan commented 3 years ago

+1 to the v1.4.x branch

davidozog commented 3 years ago

@wrrobin could you please do a review of https://github.com/openshmem-org/tests-sos/pull/16/commits/e4512e54568ac3d22cb6facfca425285cb299c23 when you have a chance? The configury is similar to SOS, but not identical. It incorporates the recent fix from https://github.com/Sandia-OpenSHMEM/SOS/pull/986

Then I'll bring in https://github.com/Sandia-OpenSHMEM/SOS/pull/988 and this PR should be good to go.

wrrobin commented 3 years ago

@davidozog The configury part looks good to me. The LDFLAGS might need an -lm as one of the tests has ceil() which is causing Travis to fail.

wrrobin commented 3 years ago

@davidozog This is ready to merge, right?

davidozog commented 3 years ago

@wrrobin It is ready, upon your approval ;)

I'll probably merge immediately after the SOS v1.5.0 release, not that it matters much...