Add community detection #1526
Add community detection #1526
Conversation
…d_community_detection # Conflicts: # pyomo/contrib/community_detection/detection.py
… model with multiple objective functions (and reformatted code)
…t the LP_unique_duals model could be used without running into an error
…s a problem with test_communities_2 that I am currently working on. For some reason, even with a seed number for the Louvain community detection, the community map generated for QCP_simple is not always the same.
…not resolve the issues I found with QCP_simple)
…r functionality is added
…e this will be handled in detection.py by the event logger; also added log_level as an argument to detect_communities and added some basic uses of the logging module
… determines whether arguments given to detect_communities are correct
…e_destination and random_seed; thus the last commit failed all test cases because None was returned instead of the community maps
…ng): - _event_log function was created (this is called in _generate_model_graph) to record information about the model and the resulting model_graph - If the file path given to detect_communities does not exist then an error will be logged along with the file path created by os.makedirs (in the _write_to_file function) - In detect_communities, important information about the decomposition/number of communities detected will be logged
…re the model is an instance of ConcreteModel 2) Added a test case that checks to make sure None is returned when given bad arguments (as well as an empty dictionary when given an empty model) 3) Removed a redundant import statement from test_detection.py
Suggested edits
| assert isinstance(model, ConcreteModel), "Invalid model: 'model=%s' - model must be an instance of " \ | ||
| "ConcreteModel" % model | ||
|
|
||
| assert type_of_community_map in ('bipartite', 'constraint', 'variable'), \ | ||
| "Invalid value for type_of_community_map: 'type_of_community_map=%s' - Valid values: 'bipartite', " \ | ||
| "'constraint', 'variable'" % type_of_community_map | ||
|
|
||
| assert type(with_objective) == bool, "Invalid value for with_objective: 'with_objective=%s' - with_objective " \ | ||
| "must be a Boolean" % with_objective | ||
|
|
||
| assert type(weighted_graph) == bool, "Invalid value for weighted_graph: 'weighted_graph=%s' - weighted_graph " \ | ||
| "must be a Boolean" % weighted_graph | ||
|
|
||
| assert random_seed is None or (type(random_seed) == int and random_seed >= 0), \ | ||
| "Invalid value for random_seed: 'random_seed=%s' - random_seed must be a non-negative integer" % random_seed | ||
|
|
||
| assert use_only_active_components is True or use_only_active_components is None, \ | ||
| "Invalid value for use_only_active_components: 'use_only_active_components=%s' - use_only_active_components " \ | ||
| "must be True or None" % use_only_active_components |
There was a problem hiding this comment.
In general, argument error checking should raise specific exceptions (usually TypeError or ValueError) instead of using the assert framework (as the assertions can be skipped if Python is run in 'optimized' mode)
| if matplotlib_available: | ||
| plt = matplotlib.pyplot |
There was a problem hiding this comment.
With the recent change to pyomo.common.dependencies, you should revert f342de2. That will prevent matplotlib from being unconditionally imported.
The way the deferred imports work, if you cast the _available flag to a bool (i.e., in an if), then the import is attempted. So, you want to avoid checking if an optional dependency is available at the module scope (i.e., when the module is imported). The deferred import system is designed so that in most cases you just grab the deferred module and use it like you normally would - it will be imported the first time it is needed.
…gument checks to raise errors
|
|
||
| if matplotlib_available: | ||
| matplotlib.use('Agg') # added to avoid $DISPLAY errors on Travis (from parmest) | ||
| matplotlib.use('Agg') # added to avoid $DISPLAY errors on Travis (from parmest) |
There was a problem hiding this comment.
Ironically, this still needs to be guarded by:
if matplotlib_available:
matplotlib.use('Agg') # added to avoid $DISPLAY errors on Travis (from parmest)Because we are in the test harness, it is OK to trigger the import check for optional modules. Further, we need to guard the call to .use, as when that is parsed, the DeferredImportModule (i.e., matplotlib) will trigger the import and then if matplotlib is not available, the attempt to return the .use attribute will generate an exception.
There was a problem hiding this comment.
Okay I see - making the changes now!
This directive is now managed centralized by pyomo.common.dependencies
Codecov Report
@@ Coverage Diff @@
## master #1526 +/- ##
========================================
Coverage 74.47% 74.47%
========================================
Files 634 638 +4
Lines 91318 91637 +319
========================================
+ Hits 68010 68251 +241
- Misses 23308 23386 +78
Continue to review full report at Codecov.
|
|
Tests are now passing (the remaining test failure is due to an intermittent GitHub infrastructure failure unrelated to this PR). @rahuljoglekar47: note that PR #1739 has also been merged into master. |
There was a problem hiding this comment.
I think this is reasonable. There are a number of things that can be improved in future iterations, including:
- @bernalde comment about embedded constants (font sizes)
- testing of the plotting capability
- conversion of the structured model generation to leverage
Reference()and avoid duplication of variables / constraints.
|
|
||
| # Make a dict of all the components we need for the NetworkX graph (since we cannot use the components directly | ||
| # in the NetworkX graph) | ||
| if with_objective: |
There was a problem hiding this comment.
This doesn't need to hold up this PR, but in future revisions, you should consider reducing repeated code with something like:
ctypes = [Constraint, Var]
if with_objective:
ctypes.append(Objective)
component_number_map = ComponentMap((component, number) for number, component in enumerate(
model.component_data_objects(ctype=ctypes, active=use_only_active_components,
descend_into=True, sort=SortComponents.deterministic)))
8 participants
Fixes # .
This PR adds a convenient way to perform Louvain community detection on a Pyomo model by automating the process of creating a NetworkX graph based on the model components.
Summary/Motivation:
The community detection package allows users to obtain a map of the communities in a model based on Louvain community detection. The user is also given a wide variety of options in how this community detection is to be performed. Lastly, using a community map like the one the main function (detect_communities) generates could be very useful as an input into some existing algorithms.
Changes proposed in this PR:
Legal Acknowledgement
By contributing to this software project, I have read the contribution guide and agree to the following terms and conditions for my contribution: