[Networkit] An effort for code sanity and cleanups / Miscellaneous / GraphIO formats

Christian Staudt christian.staudt at kit.edu
Fri Jul 31 17:32:07 CEST 2015


Hi Max,
here are my comments:

On 29 Jul 2015, at 23:23, Maximilian Vogel <maximilian.vogel at student.kit.edu> wrote:
> 
> 
> // kanboard: Code review protocol
> Often times when a task is moved to the "To Review"-column in the kanboard, nobody feels responsible to actually review the code. Therefore, I'd like to suggest the following workflow/protocol:
> When moving a task to the "To Review"-column, the developer should post a comment with the link(s) from algohub corresponding to the changesets. Then, a reviewer knows what exactly to review and doesn't need to search for the changesets. The next step would be to assign a different person to the task. The assignee will then either review the code and is supposed to post comments on algohub or look for a different person to do the review for whatever reasons.

This protocol makes sense, let’s try that in the future. First and foremost, I ask every developer to use Kanboard to plan and track their work on the project. It makes coordinating releases much easier.


> 
> // graph formats: GDF and VNA
> For the graph formats GDF and VNA only writers exists. As far as I see it, both writers are neither well coded nor up to date in terms of the supported features in the output. Sure, they can be fixed, but for both formats, it's hard to find the specifications. The gephi website has some information though.
> But the question is, if anybody even uses these formats and (solely) relies on them. Since a reader and writer for Gephi's GEXF format has been added to networkit, I think it makes more sense, to focus on a more thorough support of GEXF (and GraphML) and in general more widespread graph formats than to support a broad range of formats half-heartedly. Therefore, I vote to drop both writers.

Yes! I am in favor of dropping support for obscure file formats. We should focus on proper support for a few formats, including those that store graphs with attributes.


> 
> // auxiliary/Parallelism became useless/redundant [on the C++ side]
> Since we specify OpenMP as a "hard" requirement, the wrappers in auxiliary/Parallelism.h became rather useless, as I also noticed that they aren't used throughout networkit. Currently, the only "use" is the re-wrapping for Python in _NetworKit.pyx. But then these should be wrapped there with auxiliary/Parallelism dropped entirely.

I’m not sure I get your point. I think it’s very useful to be able to set the degree of parallelism interactively using networkit.setNumberOfThreads, which is based on Parallelism.h.

> A different approach can be seen on the clang-branch[1]: The currently used OpenMP functions are wrapped and used throughout the code base.
> Pros: Compatibility for environments where OpenMP is not available. I know that among others one focus is on implementations utilizing parallelism, but I believe that this  compatibility can be handy, even if it's only for testing purposes.

Again, I don’t see why support for environments without OpenMP is a priority at this time. However, I trust that you know how to handle the clang branch appropriately and close it eventually.

Chris


-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.ira.uni-karlsruhe.de/mailman/private/networkit/attachments/20150731/a473b8c9/attachment.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 495 bytes
Desc: Message signed with OpenPGP using GPGMail
URL: <https://lists.ira.uni-karlsruhe.de/mailman/private/networkit/attachments/20150731/a473b8c9/attachment.sig>


More information about the NetworKit mailing list