search

openshmem-org / tests-sos

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

Allow higher thread level returned at shmem init #15

Closed minsii closed 4 years ago

minsii commented 4 years ago

The SHMEM library may internally use a thread safety which is higher than that required by the user. For instance, OSHMPI always enables the async thread in order to ensure full progress. This would require the thread multiple safety even if the user requests only thread funneled.

This situation is allowed by the standard. Thus, the SOS test suite should return error only if the provided thread level is lower than the requested level.

minsii commented 4 years ago

@davidozog Can you please review? I found the issue when testing OSHMPI with SOS test suite.

jdinan commented 4 years ago

@minsii Looks good to me, although I don't think OpenSHMEM requires this comparison to be valid (it should, I will file an issue).

@davidozog Make sure you also merge these changes upstream in the main SOS repository or they will be lost in the next release sync.

minsii commented 4 years ago

@jdinan Actually I think it might be still a valid OpenSHMEM implementation if the library provides a lower thread safety than that required by the user. In other words, the spec does not require every OpenSHMEM implementation has to support the multithreading safety. Am I right?

Maybe we should allow the tests pass as long as the returned level is valid (i.e., one of the four predefined levels).

jdinan commented 4 years ago

The performance benchmarks aren't treated as a regular part of the test suite. They measure performance of specific features like threading support and contexts, so we don't assume all perf tests will work on every OpenSHMEM library. The query_thread test can raise a false negative, but if a library does not support threading, it should be disabled when configuring SOS tests.

jdinan commented 4 years ago

FYI, MPI requires that increasing thread levels have increasing integer values.

minsii commented 4 years ago

Thanks @davidozog @jdinan .

The change to OpenSHMEM spec makes sense. I was not aware the relationship SHMEM_THREAD_SINGLE < SHMEM_THREAD_FUNNELED < SHMEM_THREAD_SERIALIZED < SHMEM_THREAD_MULTIPLE is not yet defined in the current spec. Good catch!

I also agree the assumption that if the OpenSHMEM library does not support the thread level that is equal or higher than the requested level, the test should be disabled.