Add input validation #167

N-Maas · 2023-06-12T15:36:34Z

Adds validation that the input is actually a correct hypergraph (currently, KaHyPar likely segfaults in such cases or even produces garbage output). This applies to hgr files as well as the C interface and the Python interface.

This includes three major changes:

When parsing a hgr file, all (or at least most) possibilities for syntactic errors are checked and cause an appropriate error message
Adds a new utility for checking semantic errors, which is called after parsing the hgr file or when creating a hypergraph via the according interface function. All found errors as well as warnings (e.g. duplicate pins) are reported, as well as hyperedges/pins that should be ignored
Adjusts the hypergraph constructor so it can handle ignored hyperedges/pins

The required time for the semantic checks is reported in the output. The checks can be disabled using the new flag KAHYPAR_INPUT_VALIDATION. Alternatively, all warnings can be promoted to errors using KAHYPAR_INPUT_VALIDATION_PROMOTE_WARNINGS_TO_ERRORS.

readHypergraphFile now reports (almost) any syntactic error in the input file

Some of the tests expect that hyperedges of size 1 are included in the graph.

1) -lkahypar is order-sensitive and needs to be placed after the input file 2) linking boost is not necessary (already linked into the library file)

SebastianSchlag

Thanks @N-Maas. This already looks great. I haven't reviewed everything yet, but here's a first round of comments.

README.md

kahypar/application/kahypar.cc

kahypar/datastructure/hypergraph.h

kahypar/io/hypergraph_io.h

kahypar/utils/validate.h

python/tests/test_kahypar.py

tools/bookshelf_to_hgr_converter.h

codecov · 2023-06-13T15:34:43Z

Codecov Report

Merging #167 (ae56cf1) into master (e53beae) will increase coverage by 0.18%.
The diff coverage is 90.22%.

❗ Current head ae56cf1 differs from pull request most recent head 3553901. Consider uploading reports for the commit 3553901 to get more accurate results

@@            Coverage Diff             @@
##           master     #167      +/-   ##
==========================================
+ Coverage   84.42%   84.60%   +0.18%     
==========================================
  Files         204      206       +2     
  Lines       17797    18097     +300     
  Branches     9815     9945     +130     
==========================================
+ Hits        15025    15311     +286     
- Misses       2772     2786      +14

Impacted Files	Coverage Δ
kahypar/application/kahypar.cc	`0.00% <0.00%> (ø)`
kahypar/datastructure/fast_reset_flag_array.h	`91.17% <ø> (ø)`
kahypar/macros.h	`100.00% <ø> (ø)`
tools/calculate_degree_pin_distribution.cc	`0.00% <0.00%> (ø)`
tools/calculate_epsilon.cc	`0.00% <0.00%> (ø)`
tools/compute_neighborhood_sizes.cc	`0.00% <0.00%> (ø)`
tools/create_weighted_hgr.cc	`0.00% <0.00%> (ø)`
tools/evaluate_mondriaan_partition.cc	`0.00% <0.00%> (ø)`
tools/hgr_to_bipartite_graphml_converter.cc	`0.00% <0.00%> (ø)`
tools/hgr_to_bipartite_metis_graph_converter.cc	`0.00% <0.00%> (ø)`
... and 26 more

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

SebastianSchlag · 2023-07-04T17:27:40Z

include/libkahypar.h

@@ -99,8 +99,6 @@ KAHYPAR_API void kahypar_improve_hypergraph_partition(kahypar_hypergraph_t* kahy
                                                      kahypar_partition_id_t* improved_partition);


-KAHYPAR_API void kahypar_hypergraph_free(kahypar_hypergraph_t* kahypar_hypergraph);


Why do we need to remove this?

This line appeared twice in the file, which caused a compiler warning for me. Thus I removed one of the duplicates

kahypar/io/hypergraph_io.h

SebastianSchlag · 2023-07-04T17:37:35Z

kahypar/io/hypergraph_io.h

    index_vector.reserve(static_cast<size_t>(num_hyperedges) +  /*sentinel*/ 1);
    index_vector.push_back(edge_vector.size());
+    if (line_number_vector != nullptr) {


Did you mean to check if (validate_input)?

SebastianSchlag · 2023-07-04T17:38:56Z

kahypar/io/hypergraph_io.h

@@ -419,7 +500,7 @@ static inline void readFixedVertexFile(Hypergraph& hypergraph, const std::string
    }
    file.close();
  } else {
-    std::cerr << "Error: File not found: " << filename << std::endl;
+    ERROR("File not found: " << filename);


Would it make sense to use a CheckedIStream here as well?

CheckedIStream only represents a single line, so I'm not sure how one would use it here. I guess I could add another class, e.g. CheckedIFStream, which represents the complete file. But I don't know whether that is worth it

SebastianSchlag · 2023-07-04T17:39:24Z

kahypar/io/hypergraph_io.h

@@ -391,7 +472,7 @@ static inline void readPartitionFile(const std::string& filename, std::vector<Pa
    }
    file.close();
  } else {
-    std::cerr << "Error: File not found: " << std::endl;
+    ERROR("File not found.");


Would it make sense to use a CheckedIStream here as well?

kahypar/utils/validate.h

SebastianSchlag

I only got a few minor questions left. Other than those, this looks very good! 👍
Thank you very much for your efforts. I bet this will help a lot of people in the future!

N-Maas · 2023-07-06T12:35:04Z

(duplicated comment because the first is not visible in the conversation thread for me, seemingly a bug with the Github UI)

Did you mean to check if (validate_input)?

Conceptionally, yes. However, the way the function is currently written it only takes the line_number_vector parameter, which is null if no additional input validation occurs after reading the file and is otherwise used as an output parameter for mapping vertices/edges to input lines. That seemed simpler to me than adding even more parameters

SebastianSchlag · 2023-07-11T14:31:40Z

Thank you very much, @N-Maas! 👍

N-Maas added 15 commits June 12, 2023 14:20

add missing include and fix some warnings

19417f5

validate hmetis input file

438eb36

readHypergraphFile now reports (almost) any syntactic error in the input file

add input validation utility

d4d5e48

add test for input validation

1d4bf9c

integrate input validation to hypergraph_io

a223b9e

hypergraph construction and (first) test

8c51485

help message for sentinel element

e6579d4

add additional hypergraph construction test

a195faa

disable input validation in conversion tests

be0aa80

Some of the tests expect that hyperedges of size 1 are included in the graph.

correct handling of unexpected end of file during parsing

3ffdf04

integrate input validation to C interface

10e7e71

README: fix and simplify command for compiling with C interface

00632f1

1) -lkahypar is order-sensitive and needs to be placed after the input file 2) linking boost is not necessary (already linked into the library file)

add function for explicit input validation to C interface

5591dbe

integrate input validation to python interface

bae6130

README: add section for input validation

4ace405

N-Maas linked an issue Jun 12, 2023 that may be closed by this pull request

Proposal: Input validation #166

Closed

SebastianSchlag reviewed Jun 13, 2023

View reviewed changes

N-Maas added 2 commits June 13, 2023 16:51

review minor

72fb1e5

don't use default arguments for createHypergraphFromFile

9de6a52

N-Maas mentioned this pull request Jul 4, 2023

Fix Titan23 Instances #172

Merged

SebastianSchlag reviewed Jul 4, 2023

View reviewed changes

kahypar/io/hypergraph_io.h Outdated Show resolved Hide resolved

SebastianSchlag reviewed Jul 4, 2023

View reviewed changes

kahypar/utils/validate.h Outdated Show resolved Hide resolved

SebastianSchlag approved these changes Jul 4, 2023

View reviewed changes

SebastianSchlag mentioned this pull request Jul 4, 2023

Stuck during coarsening #107

Closed

use ERROR macro more consequently

fd1d8c5

remove unnecessary parameter from printErrors

3553901

SebastianSchlag merged commit 38ef99b into kahypar:master Jul 11, 2023
3 of 4 checks passed

SebastianSchlag mentioned this pull request May 31, 2024

Can KaHyPar support zero vertex weight? #202

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add input validation #167

Add input validation #167

N-Maas commented Jun 12, 2023

SebastianSchlag left a comment

codecov bot commented Jun 13, 2023 •

edited

SebastianSchlag Jul 4, 2023

N-Maas Jul 6, 2023

SebastianSchlag Jul 4, 2023

SebastianSchlag Jul 4, 2023

N-Maas Jul 6, 2023

SebastianSchlag Jul 4, 2023

N-Maas Jul 6, 2023

SebastianSchlag left a comment

N-Maas commented Jul 6, 2023 •

edited

SebastianSchlag commented Jul 11, 2023

		@@ -99,8 +99,6 @@ KAHYPAR_API void kahypar_improve_hypergraph_partition(kahypar_hypergraph_t* kahy
		kahypar_partition_id_t* improved_partition);


		KAHYPAR_API void kahypar_hypergraph_free(kahypar_hypergraph_t* kahypar_hypergraph);

Add input validation #167

Add input validation #167

Conversation

N-Maas commented Jun 12, 2023

SebastianSchlag left a comment

Choose a reason for hiding this comment

codecov bot commented Jun 13, 2023 • edited

Codecov Report

SebastianSchlag Jul 4, 2023

Choose a reason for hiding this comment

N-Maas Jul 6, 2023

Choose a reason for hiding this comment

SebastianSchlag Jul 4, 2023

Choose a reason for hiding this comment

SebastianSchlag Jul 4, 2023

Choose a reason for hiding this comment

N-Maas Jul 6, 2023

Choose a reason for hiding this comment

SebastianSchlag Jul 4, 2023

Choose a reason for hiding this comment

N-Maas Jul 6, 2023

Choose a reason for hiding this comment

SebastianSchlag left a comment

Choose a reason for hiding this comment

N-Maas commented Jul 6, 2023 • edited

SebastianSchlag commented Jul 11, 2023

codecov bot commented Jun 13, 2023 •

edited

N-Maas commented Jul 6, 2023 •

edited