fix unweighted flag saving

[flomlo]

Reference Issues/PRs

Example: Fixes #58 partially, does not add test coverage yet

What does this implement/fix? Explain your changes.

now looks more like save_unweighted_flag. also works.

[ulupo]

This is a portion of the code I am not very familiar with unfortunately. @flomlo could you perhaps explain the changes, why the previous version failed, etc? Also it would be good to add a little regression tests as discussed in #58. It can be quite simple. I can help you integrate it in the testing suite.

[flomlo]

Hi! Sorry, haven't noticed before that the discussion moved here.

@ already submitted code: I've modified the save_unweighted_flag routine to make the output of it look more like e.g. e.flag in the flagser repo: https://github.com/luetge/flagser/blob/7ffe92782ae639b4265a52403d2d90d3151bb67b/test/e.flag

There are two changes:

• vertices is reshaped so that it forms a line vector instead of a column vector. This is analogous to save_weighted_flag. If one does not do this, the outputted flag file is corrupted and it complains about not enough vertices being available for the respective edges.

• edges are saved with fmt='%i %i %i' instead of fmt='%i %i. Here the first integer is i, the second j and the third the weight w_ji. Well, the original version simply did not work, as edges has three columns.

Note that the resulting .flag files have 1 as weights, not 0. That does not matter, though.

[flomlo]

Regarding the test cases:

For some reason only save_weighted_flag is tested. I think just copying that to save_unweighted_flag is good enough. I do not really understand why the current testing code is written like it is written, so I would prefer to not modify it.

[ulupo]

@flomlo thanks for submitting this! Let me add the test then, and I'll merge.

[ulupo]

@flomlo I notice that you made your changes in your master branch and created a PR from there. I know it seems overly pedantic to say this, but this is strongly recommended against. A separate "feature branch" should be created with the changes and the PR should come from there. master in a fork should always be in sync with the origin repository. Feel free to either close this PR and reopen a new one from such a feature branch, or to try one of the solutions in https://coding.abel.nu/2015/03/fixing-a-pull-request-from-master/ (probably not worth the trouble in this case). Sorry for being annoying!

[flomlo]

regarding tests:

I would have written something which checks that (for a random graph g) the following holds: load(save(g)) = g. (This should be true both for weighted and unweighted.)

The testsuite does other stuff, and I'm not sure why

[flomlo]

@flomlo I notice that you made your changes in your master branch and created a PR from there. I know it seems overly pedantic to say this, but this is strongly recommended against. A separate "feature branch" should be created with the changes and the PR should come from there. master in a fork should always be in sync with the origin repository. Feel free to either close this PR and reopen a new one from such a feature branch, or to try one of the solutions in https://coding.abel.nu/2015/03/fixing-a-pull-request-from-master/ (probably not worth the trouble in this case). Sorry for being annoying!

Yeah, I'm git-noob :D I'll check on it after lunch break, ok?

[ulupo]

@flomlo no worries!

[flomlo]

Oh, and another quick note regarding the tests: I'm not sure about the expected behaviour of saving/loading of graphs with zero edges. This might be an edge-case to check for (or zero-edge-case?)