Skip to content

Fix issue with Database class #276

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.

Sign up for GitHub

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

Merged
ckendrick merged 5 commits into from
May 16, 2024
Merged

Fix issue with Database class #276

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
a5d336c
Add test for writeSnapshot/endSamples in CI
ckendrick Mar 11, 2024
60b1f15
Fix test return value
ckendrick Mar 11, 2024
c62ec3b
Remove unused status variable
ckendrick Mar 11, 2024
0ac7640
Fix bug with assert statements in BasisReader
ckendrick Mar 26, 2024
c872267
Remove unused function
ckendrick May 9, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .github/workflows/run_tests/action.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ runs:
./tests/test_Vector
./tests/test_Matrix
mpirun -n 3 --oversubscribe ./tests/test_Matrix
./tests/smoke_static
./tests/test_DEIM
./tests/test_GNAT
./tests/test_QDEIM
Expand Down
6 changes: 3 additions & 3 deletions lib/linalg/BasisReader.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@ BasisReader::getSpatialBasis(
int num_cols = getNumSamples("basis");

char tmp[100];
CAROM_VERIFY(0 < start_col <= num_cols);
CAROM_VERIFY(0 < start_col && start_col <= num_cols);
CAROM_VERIFY(start_col <= end_col && end_col <= num_cols);
int num_cols_to_read = end_col - start_col + 1;

Expand Down Expand Up @@ -177,7 +177,7 @@ BasisReader::getTemporalBasis(
int num_cols = getNumSamples("temporal_basis");

char tmp[100];
CAROM_VERIFY(0 < start_col <= num_cols);
CAROM_VERIFY(0 < start_col && start_col <= num_cols);
CAROM_VERIFY(start_col <= end_col && end_col <= num_cols);
int num_cols_to_read = end_col - start_col + 1;

Expand Down Expand Up @@ -351,7 +351,7 @@ BasisReader::getSnapshotMatrix(
int num_rows = getDim("snapshot");
int num_cols = getNumSamples("snapshot");

CAROM_VERIFY(0 < start_col <= num_cols);
CAROM_VERIFY(0 < start_col && start_col <= num_cols);
CAROM_VERIFY(start_col <= end_col && end_col <= num_cols);
int num_cols_to_read = end_col - start_col + 1;

Expand Down
6 changes: 0 additions & 6 deletions lib/utils/Database.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,12 +16,6 @@

namespace CAROM {

bool fileExists(const std::string& name)
{
std::ifstream f(name.c_str());
return f.good();
// ifstream f will be closed upon the end of the function.
}

Database::Database()
Comment thread
ckendrick marked this conversation as resolved.
{
Expand Down
1 change: 0 additions & 1 deletion lib/utils/Database.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@

namespace CAROM {

bool fileExists(const std::string& name);

/**
* Class Database is an abstract base class that provides basic ability to
Expand Down
4 changes: 1 addition & 3 deletions unit_tests/smoke_static.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,8 +53,6 @@ main(
return 1;
}

bool status = false;

// Define the values for the first sample.
double vals0[6] = {1.0, 6.0, 3.0, 8.0, 17.0, 9.0};

Expand Down Expand Up @@ -144,5 +142,5 @@ main(

// Finalize MPI and return.
MPI_Finalize();
return !status;
Comment thread
dylan-copeland marked this conversation as resolved.
return 0;

@dylan-copeland dylan-copeland May 10, 2024 •
edited
Loading

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks like the goal of this test is just to run without failing? There is nothing checked, like in test_Matrix.cpp where EXPECT* functions are called. Is this the intention?

@ckendrick ckendrick May 16, 2024

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes this test was added to the CI only intending to be a quick check if an error occurred with endSamples()/writeSnapshot(). The issue that caused this problem has since been fixed, but the only test I could find that uses these functions is in test_HDFDatabase.cpp which is restricted to parallel HDF5 builds only.
In the future, we should provide a serial test in test_HDFDatabase.cpp and remove this test (and other outdated/unused tests).

}