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

Error when using queries with the BP5 engine #3707

Closed
anagainaru opened this issue Jul 21, 2023 · 7 comments
Closed

Error when using queries with the BP5 engine #3707

anagainaru opened this issue Jul 21, 2023 · 7 comments

Comments

@anagainaru
Copy link
Contributor

If I try to use a query on a BP file created with BP5 I get errors about variables not existing.

To reproduce the error, use the BP5 engine in the testing code:

diff --git a/testing/adios2/performance/query/TestBPQuery.cpp b/testing/adios2/performance/query/TestBPQuery.cpp
index ce6eed40a..6f2971879 100644
--- a/testing/adios2/performance/query/TestBPQuery.cpp
+++ b/testing/adios2/performance/query/TestBPQuery.cpp
@@ -241,7 +241,7 @@ void BPQueryTest::WriteFile(const std::string &fname, adios2::ADIOS &adios,

 TEST_F(BPQueryTest, BP3)
 {
-    std::string engineName = "BP3";
+    std::string engineName = "BP5";
     // Each process would write a 1x8 array and all processes would
     // form a mpiSize * Nx 1D array
     const std::string fname(engineName + "Query1D.bp");

When running it, I get this error:

[Fri Jul 21 05:12:38 2023] [ADIOS2 ERROR] <Query> <XmlWorker> <ParseVarNode> : No such variable: doubleV
unknown file: Failure
C++ exception with description "[Fri Jul 21 05:12:38 2023] [ADIOS2 EXCEPTION] <Toolkit> <query::XmlWorker> <ParseVarNode> : variable: doubleV not found
: unspecified iostream_category error" thrown in the test body.
@anagainaru anagainaru changed the title Defining queries for the BP5 engine Error when using queries with the BP5 engine Jul 21, 2023
@eisenhauer
Copy link
Member

One significant difference between reader-side BP3 and BP5 is that BP5 doesn't load all variable metadata on Open(). In particular, the default Open mode (Mode::Read, which requires Begin/EndStep), it doesn't load that information until the first BeginStep(). BP3 used to load all the variable information immediately, and then you could either do BeginStep, or start accessing steps randomly using SetStepSelection(). That was flexible, but performance-wise it made Open hugely expensive at scale and was particularly wasteful if you were just going to access data step-by-step. BP5 changed that and only does metadata load immediately if you use Open(Mode::ReadRandomAccess). We had to change a lot of tests and add a mention in the release notes because this does break old code, but it was deemed necessary. For Query in general it may mean a more significant redesign, but you can make the test work by just adding the mode specifier. (I'm surprised this didn't come up earlier. We tried to kill the explicit SetEngine("bp3") in everything in the "testing" directory, but I wasn't aware that there were any regression tests in the "performance" directory. But there were discussions of this change in the dev meetings too of course.)

@anagainaru
Copy link
Contributor Author

That makes sense. I'll change the testing to use BP5 with Open(Mode::ReadRandomAccess) so we have something working, but we should bring this up in the dev meeting to design a solution for the future.

I would also like queries to be able to see the min/max of sub-blocks. I was using statsblocksize to create more sub-block metadata and I was surprised when the query was only seeing one block.

@anagainaru
Copy link
Contributor Author

Update: I still see the error when I open the file with Mode::ReadRandomAccess

@eisenhauer
Copy link
Member

Interesting... I'll take a look in a bit.

@eisenhauer
Copy link
Member

There are two places where a file gets opened with Mode::Read and when I replace both of them with ReadRandomAccess, the error above is replaced with :

C++ exception with description "[Sat Jul 22 11:14:25 2023] [ADIOS2 EXCEPTION] <Engine> <BP5Reader> <BeginStep> : BeginStep called in random access mode
" thrown in the test body.

Unfortunately as written, the tests won't work in BP5 either mode because BeginStep is explicitly forbidden if you use ReadRandomAccess mode and the metadata isn't loaded on open in Read mode... So we really have to bite the bullet and rethink this a bit for it to work with the newer BP5 variable semantic.

@anagainaru
Copy link
Contributor Author

I think this is resolved. @guj let me know if not, otherwise I'll close this issue

@guj
Copy link
Contributor

guj commented Dec 6, 2023

yes, please close it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants