ornladios
Engine state getter
It would be very valuable to provide users with an object that describes a state of an engine. Mainly, it should retrieve the information if begin step was executed and end step was not. Internally in ADIOS2 a variable is used defined in Engine.h
/** true: Currently executing after BeginStep and before EndStep */
bool m_BetweenStepPairs = false;
This variable should be accessible from C API.
Let me push back with a little nuance. We do have that m_BetweenStepPairs in a few engine subclasses (SST and BP5 read and write, BP3/4 reader only, no supported in the engine class itself). In general it is only used so that we can throw a friendly exception if the user screws up: calls beginstep twice in a row or calls endstep without a beginstep. It also helps handle a historical oddity where for BP writer engines it has been acceptable to do Open(), Put(), Close() without ever doing a BeginStep/EndStep pair. This is a very old ADIOS1 coding pattern, but still not officially deprecated (as in - we still have tests that work that way).
I would probably argue against adding access to this internal variable for several reasons.
- For one thing, it's easy for the user to implement BeginStep/EndStep tracking in their own code if they need it simply by having their own boolean.
- The user could also simply call BeginStep() and if they'd already done it they could catch and ignore the logic_error() exception. For the C API we're already trapping exceptions and converting them into error return values of the enum type adios2_error. I don't know what we'd return in this case, but I think we could easily extend adios2_error to be unambiguous in this circumstance and cover this functionality.
- Lastly, our API is cluttered with things already. This isn't supported on all engines, is unnecessary for most applications, and has easy alternatives for getting the same effect using the existing API. Not that we might not in the future need to access some engine state externally, but I think this particular use isn't compelling..
IMHO, it may be easier for an application, especially a middleware between application and ADIOS, to just ask the Engine itself if it is inside a Step to avoid calling it twice. Tracking an external boolean independent from the engine object somewhere in the application/middleware just adds a bit more complexity.
But as you explain this would require every engine to properly track this both on write and read side.
I also strongly believe that the tracking should be on the library side.
So, I have created a PR: https://github.com/ornladios/ADIOS2/pull/3216 But there are caveats. m_BetweenStepPairs was already an top level Engine-class member, available to all engine subclasses to update. But, many of them do not. It would probably make sense to make Begin/EndStep to exist only in the Engine class, have it do some bookkeeping and then call DoBeginStep/DoEndStep in the engine subclasses. That way we could be assured that all engines, for example, update the BetweenStepPairs member, raise an exception if you call BeginStep twice, etc. However, this change is more complex than just renaming every appearance of Begin/End step in the engine subclasses to DoBegin/DoEndStep. Part of the problem is that some operations (like Close()) implicitly do some of those things, etc. So, trying to make this all nice and clean runs straight into existing messiness that is mostly unique to each engine. The alternative, to change each engine that doesn't currently maintain the m_BetweenStepPairs member to do it appropriately and simply live with their other idiosyncrasies is more tractable. We can talk about which approach makes more sense, but for now I'm just doing the bindings work for C and C++ to expose the existing Engine member.
What's the difference between checking if(engine.BetweenStepParis == true) and checking if(engine.BeginStep() != 0) ?
I don't understand. Will this difference block any workflows?
