-
Notifications
You must be signed in to change notification settings - Fork 51
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
Opt-in support for unknown ADIOS2 engines #1652
Conversation
3f82b1c
to
100bf34
Compare
Is this PR intending to allow new additional adios engine types (other than the ones defined in the list)? It seems to me that the function fileSuffix() still throws error when engine type (from either definitions) is not in the defined list. |
Yes. The idea is that new engines which we don't support (like the currently experimental Campaign engine, or other engines like Dataman) can be experimentally used by telling the ADIOS2 backend to treat the engine like one of the supported engines.
The logic of std::string ADIOS2IOHandlerImpl::fileSuffix(bool verbose) const
{
// ... some defines ....
if (m_realEngineType.has_value())
{
// unknown engine type, use whatever ending the user specified
return m_userSpecifiedExtension;
}
// .... So this should not be a problem |
Thanks Franz. As the engine type is also able to be defined as env variable "OPENPMD_ADIOS_ENGINE", should there be an equivalent env for "treat_unsupported_engine_like" as well? I understand the json config takes priority over env variables. |
engineConfig | ||
[adios_defaults::str_treat_unsupported_engine_like] | ||
.json()); | ||
if (!maybeEngine.has_value()) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess a boolean flag would also do? "e.g. accept_unspported_engine=true"?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If true, can always return ".bp" at realEngineType(). The actual value of the treat_supported_engine expected to be one of the known adios engines, and exactly which one is not relavent I think.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess a boolean flag would also do? "e.g. accept_unspported_engine=true"?
That will not be enough because there is no "standard" way how an engine should be treated. Especially, most engines differ slightly in how steps should be treated (staging engines require steps, BP5 requires steps with normal Read mode, but cannot deal with steps in ReadRandomAccess, the Campaign engine fails when using steps in any way, other engines are mostly a mixture).
I don't think that providing one generic implementation for an unknown future engine will work, it seems better to use the existing implementations and let users try which works best.
If true, can always return ".bp" at realEngineType(). The actual value of the treat_supported_engine expected to be one of the known adios engines, and exactly which one is not relavent I think.
realEngineType()
will return the Engine type that is actually passed on to ADIOS2. m_engineType
determines the behavior of the ADIOS2 backend. Especially BP5 behaves slightly different from the other BP engines.
I don't see a reason to put extra work into restricting the options here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure. Thanks
9df26a4
to
8533aeb
Compare
I can add that, yeah |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Awesome, LGTM :)
The ADIOS2 backend has a set of known and supported ADIOS2 engines. The consequence is that new, unknown or unsupported engines cannot be used without changing the code of the ADIOS2 backend.
This PR adds the backend key
adios2.engine.treat_unsupported_engine_like
in order to allow users to try alternative backends at their own risk.e.g.:
TODO:
Notes to myself for documenting advanced features not supported by this:
QueueLimit
filename extension handling: default extensions, mandatory extensions in BP3 and SST
Span API
PerformDataWrite