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.

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

Jump to bottom

Metis Integration and Optional Libraries #153

Merged
merged 13 commits into from
Sep 21, 2022
Merged

Metis Integration and Optional Libraries #153

merged 13 commits into from
Sep 21, 2022

Conversation

ardasener
Copy link
Contributor

This PR will provide a MetisPartition class in the preprocessor module which is a wrapper for METIS.
Also a PartitionPreprocessType class is established, intended to be used by all future partition classes.

There are also more substantial changes introduced in this PR:

I will make a separate PR soon to add documentation regarding these changes.

ardasener and others added 10 commits September 9, 2022 09:40
@ardasener
Started working on metis integration
1a1feea
@ardasener
Added the new system for adding optional libraries, started revamping…
ad5330d 
… generate_explicit_instantiations script
@ardasener
Added parameters and example for metis, instantiation script is compl…
f897a35 
…ete except a few edge cases
@ardasener
Small tweaks
03c3de7
@AmroAlJundi
Fixed bug in EdgeListReader
4f488cb
@ardasener
Changed the CMake optional library system to also use environment var…
6315833 
…iables and default prefix locations
@ardasener
Merge remote-tracking branch 'origin/develop' into feature/metis_inte…
64cde22 
…gration

# Conflicts:
#	examples/CMakeLists.txt
#	src/generate_explicit_instantiations.py
#	src/sparsebase/preprocess/preprocess.h
@ardasener
Merged with developed and resolved conflicts
108e23c
@ardasener
Fixed ConverterMixin not being instantiated
2bfd4ad
@ardasener
Removed ifdef from PIGO instantiations
5261702
This was linked to issues Sep 19, 2022
Design a system for adding optional dependencies to the library #140
Closed
Better Instantiation Generation Script #141
Closed
@ardasener ardasener added priority: now Critical priority state: review needed type: feature Brand new functionality, features, workflows, endpoints, etc labels Sep 19, 2022
@ardasener ardasener added this to the Milestone 3 milestone Sep 19, 2022
AmroAlJundi
AmroAlJundi previously approved these changes Sep 20, 2022
View reviewed changes
@ardasener ardasener linked an issue Sep 21, 2022 that may be closed by this pull request
Add METIS Partitioning #162
Closed
AmroAlJundi
AmroAlJundi approved these changes Sep 21, 2022
View reviewed changes
Copy link
Contributor

@AmroAlJundi AmroAlJundi left a comment

Choose a reason for hiding this comment

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

Looks good. Just two comments:

  • getting started page should be updated?
  • also about the return of number of parts (by the way, it can also be a utility function in PartitionBase that counts it in O(n) time).

src/sparsebase/preprocess/preprocess.h
/*!
* @returns An IDType array where the i-th index contains the ID for the partitioning i belongs to
*/
IDType* Partition(format::Format * format, std::vector<context::Context*> contexts, bool convert_input);
Copy link
Contributor

Choose a reason for hiding this comment

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

Actually, i was thinking about implementing the class that will apply a partitioning, and maybe it would be useful for partitioning to also return the number of parts. What do you think?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Great idea. But lets open a separate issue for this, this PR waited long enough :)

src/sparsebase/preprocess/preprocess.h
* Wraps the METIS partitioner available here: https://github.com/KarypisLab/METIS
* The library must be compiled with the USE_METIS option turned on
* and the pre-built METIS library should be available.
* See the Optional Dependencies page (under Getting Started) in our documentation for more info.
Copy link
Contributor

Choose a reason for hiding this comment

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

It seems there aren't any updates to the getting started page in this PR. will this be added later or did you forget to add the file 😂

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have another branch with that file added. But I think I didn't intend to write this reference before that merged. For some reason I forgot that and added it in. I will make another PR soon with the updated docs.

@AmroAlJundi AmroAlJundi added state: approved Approved to proceed. Ready to be merged and removed state: review needed labels Sep 21, 2022
@ardasener ardasener merged commit 424f963 into develop Sep 21, 2022
@ardasener ardasener deleted the feature/metis_integration branch September 21, 2022 16:31
@ardasener ardasener mentioned this pull request Sep 26, 2022
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@AmroAlJundi AmroAlJundi AmroAlJundi approved these changes

Assignees
No one assigned
Labels
priority: now Critical priority state: approved Approved to proceed. Ready to be merged type: feature Brand new functionality, features, workflows, endpoints, etc
Projects
None yet
Milestone
Milestone 3
2 participants
@ardasener @AmroAlJundi