My First Experiences on Raising a ns3-dev Merge-Request at GitLab

[Charles Pandian]

In this post, I want to share my first experience on 'Raising a simple merge request at GitLab' at https://gitlab.com/nsnam/ns-3-dev/-/merge_requests

You may find that specific request at - A Simple CircleMobilityModel (3D) for ns-3.

I want some advice/comments and your feedback in this regard. Your simple comment in your reply may help me and other new contributors to do things correctly in future.

Note: if you have trouble in reading this mail here (because of a lot of inline images in this mail) , you may see my original post here where I am maintaining it: My First Experience on Raising a ns3-dev Merge-Request at GitLab

Contributing to ns-3

In fact, I want to incorporate the simple CircleMobilityModel that I developed for ns-3 with the ns-3-dev; so that the future distributions of ns-3 will contain my mobility model in the default installation of ns-3.

You may find the internal mechanism of my CircleMobilityModel in the following post :

Implementation of Circle Mobility Model for ns-3 and Visualizing it in 3D

 

I tried to learn the procedure of submitting a new contribution to ns-3 while writing an article on it.  The following is the link to that article :

Confusions in Contributing Our New Model to mainline ns-3. 

 

After making the model to ready for submission, I made a submission at ns3-dev with a comment explaining the model.

 

The following is the actual content of the comments that I made along with that merge request:

 

A Simple CircleMobilityModel (3D) for ns-3

I wish to add a Simple CircleMobilityModel (3D) in ns-3. I wrote this model by cloning the skeleton structure of ns-3's "ConstantPositionMobilityModel" by Mathieu Lacage. Only a few standard declaration lines are directly handled from ConstantPositionMobilityModel code, and mostly, 90% of lines of this models were only rewritten by me. This model uses circle functions inspired from Circle Function Logic of Circle Mobility Model of Omnet++ by Andras Varga. The original Omnet++ version used radians and degree conversions in its implementation; but I handled angles only in degree.

This merge will add "circle-mobility-model.h" and "circle-mobility-model.cc" file under "src/mobility/models" and do the necessary modifications in wscript and CMakeLists.txt and other files to incorporate CircleMobilityModel in ns-3-dev. This will also add two simulation example scripts under src/mobility/examples and add the necessary test script under src/mobility/test. It will do the necessary changes in src/mobility/test/examples-to-run.py and src/mobility/examples/wscript to incorporate them with in ns-3-dev.

The following are the NetAnim outputs (captured as gif animation) for our example scripts : simple-3d-circle-mobility-example1.cc and simple-3d-circle-mobility-example2.cc

The first one is an Example with one node with CircleMobilityModel (simple-3d-circle-mobility-example1.cc).

 

The second one is an Example with one node with CircleMobilityModel along with 50 nodes having GaussMarkovMobilityModel (simple-3d-circle-mobility-example2.cc).

In fact, the above are 2D projections of 3D Network simulations on NetAnim, so that the output in 3D can be visualized using ns-3 extensions such as NetSimulyzer (this merge is not providng with NetSimulyzer version of simulation script since ns-3-dev doesn't have those required modules/model).

The following is one such possibility of visualizing this ns-3 CircleMobilityModel Simulation with NetSimulyzer 3D visualization Tool. In the following examples, the boat moving (in the reverse direction? :-) ) on the water surface is using the proposed CircleMobilityModel.

Since the merge request was filed because of some minor errors that I made in the "comments" of code (No other syntax or logical errors were reported by that automated merge process), I tried to get Mr Tom in this regard.

My Mail to Mr. Tom

 

The following is the mail content that I sent to Mr.Tom for getting his help and comments on it :

Dear Mr. Tom,

I started to learn GitLab way of doing things. :-) 
(Still, I am unable to understand it even for doing basic things - later I may try to post an 'idiots guide for using GitHub for contributing to ns-3-dev if I will be successfully merging my MobilityModel with ns-3-dev)

ok.

I forked ns-3-dev and incorporated the necessary things to add my CircleMobilityModel in it. (after testing the same in a cloned ns-3-dev in my local drive)

I raised a merge request for my first contribution to ns-2.
https://gitlab.com/nsnam/ns-3-dev/-/merge_requests/821

First Merge Attempt:
In fact, for the first time, the pre-commit-compile-*  successfully compiled and tested everything.
But, while compiling documentation, except Doxygen, the other three got succeeded.

 Doxygen failed because of errors like :

src/mobility/model/circle-mobility-model.h:66: warning: argument 'void' of command @param is not found in the argument list of ns3::CircleMobilityModel::DoGetPosition(void) const
src/mobility/model/circle-mobility-model.h:68: warning: return type of member ns3::CircleMobilityModel::DoGetPosition is not documented

This process also consumed around 2 hours

Second Merge Attempt:
To resolve the errors in those comments, I only "editor" that particular comments in those files and retried the merge.

 

It started another fork of my project with those modified files and again started to do pre-commit-compile-* and then Doxygen. 
And failed at Doxygen with the same kind of 'comment' errors.
This process also consumed around 1.5 hours

Third Merge Attempt:
I removed that  (unexpected) fork, and resolve the errors in those comments, I uploaded edited versions of the same files (from local disk) and retried the merge.

 

It again started to do pre-commit-compile-* and will compile the Doxygen. 
And it may fail at Doxygen with the same kind of 'comment' errors.
This process not yet completed. But I think, it will also consume around an hour.

My Questions are:
1. Why Doxygen is not failing on other ns-3 header files where there were no Doxygen comments at all?.

2. Why it is running the all the test of other models again and again at the end  of  all pre-commit-compile-* ? 

3. Is this the 'normal' usual way we have to do things while merging after a "single line change in the comment line of the code?" 

    (I mean waiting for more than an hour for editing a single line of comment? :-) :-) :-) )

4. Is there any other way to quickly do the  pre-commit-compile-*  and Doxygen compile only on the edited files?

For your information, I think, I am provided with a "shared pipeline" (I have no idea about it) since I am using a free account.

the following image shows the running process of  pre-commit-compile-* after one line editing.

 

Note: if you insist me,  then  I may post my future questions at the group.

Charles Pandian,

Reply from Mr. Tom

After carefully examining the problems that I faced during submission (the merge request), Mr. Tom posted me the following reply :

Mr.Tom's reply highlighted in 'Bold Green Text'

 

On 12/18/21 4:38 AM, Charles Pandian wrote:
> Dear Mr. Tom,
>
> I started to learn GitLab way of doing things. :-) 
> (Still, I am unable to understand it even for doing basic things - later
> I may try to post an 'idiots guide for using GitHub for contributing to
> ns-3-dev if I will be successfully merging my MobilityModel with ns-3-dev)
....

.....

....

......
> My Questions are:
> 1. Why Doxygen is not failing on other ns-3 header files where there
> were no Doxygen comments at all?.

The testing infrastructure only checks the files that have been touched
in the merge request.

> 2. Why it is running the all the test of other models again and again at
> the end  of * all pre-commit-compile-* ?
> *
> 3. Is this the 'normal' usual way we have to do things while merging
> after a "*single line change in the comment line of the code*?"
>      (I mean waiting for more than an hour for editing a single line of
> comment? :-) :-) :-) )

I would like to raise this issue with the maintainer who is more of an
expert on this CI behavior (Tommaso Pecorella).

> *
> 4. Is there any other way to quickly do the  pre-commit-compile-*  and
> *Doxygen compile only on the edited files?

I am not sure whether there is a lighter weight way to run the Doxygen
only on the changed files.

>
> *_For your information, I think, I am provided with a "shared pipeline"
> (I have no idea about it) since I am using a free account._*

I personally do all of my testing on a local machine and ignore what the
GitLab CI is doing, because it is slow and because I personally do not
like the YAML framework.  However, I think we need to make sure that
contributors such as you do not have a bad experience (lengthy delays)
if they are relying on the shared pipelines.

Do you mind if I forward your mail to Tommaso Pecorella (and cc you)?

- Tom

I posted the following update of status on this to Mr. Tom before getting the above reply. But I think he didn't notice them since I send them as an inline reply. Anyway, If I get any comments in this regard, I will post them here.

 

Dec 18, 2021, 8:09 PM
Charles Pandian <igs...@gmail.com> to Tom

Update :

After almost 1.5 hours,  the merge failed again  with the following errors:

src/mobility/model/circle-mobility-model.h:67: warning: return type of member ns3::CircleMobilityModel::DoGetPosition is not documented3840
src/mobility/model/circle-mobility-model.h:74: warning: documented empty return type of ns3::CircleMobilityModel::DoSetPosition3841
src/mobility/model/circle-mobility-model.h:79: warning: return type of member ns3::CircleMobilityModel::DoGetVelocity is not documented3842
src/mobility/model/circle-mobility-model.h:84: warning: documented empty return type of ns3::CircleMobilityModel::UpdateCourseChange

 

Why it is getting forked for each and every change in one file?

 

Why it is compiling and testing everything for each and every change in one file?

 

 

Of course, I may understand these errors related ith Doxygen and will correct them.

But the question is :
Why it is compiling and testing every modules/models even though we are only adding one module.
(of course, the first waf will do the full compile - but why the consecutive wafs are doing everything again and again?)


So, I  almost spent more than 8 hours just for correcting a error in a "comment" statement.
It is very strange.

Is it the normal way of using GitLab, or am I doing it in an idiotic way?

Charles Pandian,

Another Update on this

 

Dec 18, 2021, 10:59 PM
Charles Pandian <igs...@gmail.com> to Tom

Mr. Tom,Just sharing my first experience with GitLab and ns-3-dev merge requests.In fact, almost all of the following failure is only because of some small errors in Doxygen Comments on one particular file.In fact, while compiling it in my local repository, it Doxygen compile was successful.In fact, I started my first merge request on this morning at 8:59 am.

My last failure was at 10:43 pmAlmost I spent 14 hours just for correcting a few mistakes that I made on a few lines of comments in two files. ( not a single mistake in the code)Still, the merge is under process. I hope that I will finish successfully this time.

Lots of good ns-3 extensions that are not yet incorporated with mainline ns-3-dev.

These kinds of obstacles in the merge process may be a reason for the very slow growth of ns-3.

But, still, I believe that my poor knowledge of git/GitLab and ns-3 contribution standards are causing these problems and this much of delay for correcting a few lines of comments.

Kindly share your comments and advice on this probem.

 

Charles Pandian,

 

Finally, the merge request was successful (not the merge itself - the merge is now waiting for review)

I posted the following message to Mr. Tom

 

Dec 19, 2021, 12:46 PM
Charles Pandian <igs...@gmail.com> to Tom

Dear Mr. Tom,Thank you for recognizing my little first contribution to ns-3-dev and your encouraging words.Just I want to share my happiness with you.At last, I received the message, "Pipeline has been fixed and #432474760 has passed!" at 12.47 am.So, my merge request was successful after  16 hours of trying it.

The fact is: I only spent that 16 hours only to correct a few lines of "comments" in one ".h" file and one ".cc" file.Of course, I can understand that you and your team will explore this and
(1) may find the mistake that I made during my merge request

(2)  or may find some flaw in the GitLab UI of ns-3-dev during pre-commit-compile-* and other pre-commit document compile including Doxygen Compile.  

(3) or may find the limitations of using a free pipeline of GitLab.
(4) or may provide some configuration settings on the forked ns-3-dev project on GitLab; so that the merge request trials may end in at least within 2 hours (even it is necessary to edit a few lines, a few times like in my case).

(5) you and others contributed may use some other "direct" mechanism to do things so that you may unaware of this problem.

(6) or, we, the people who use GitLab from the different geographical regions may only have this kind of poor experience with GitLab because of the poor "free pipeline" (in fact, I am not at all understand what is a pipeline :-) :-) )

Of course, I understand that the review process also may take a lot of time because of the workload of the developers/reviewers and again may require another 16 hours :-) :-)  for me to correct things according to the feedbacks and comments of the reviewers through the mechanisms provided through GitLab.

I hope that your comments and feedback will be useful for every beginner who wants to contribute something to ns-3.

My little suggestion of re-engineering the pre-commit-compile-*Process:

The pre-commit compile is running as 4 separate processes.

So, if one made a "syntax error" in code, then these 4 pre-commit compile possesses will just waste CPU time in four threads and will end up with the same error after 2 hours.
If we allow only one thread (I mean only one pre-commit compile process at one time) at full speed, then it will end with the same error after a few minutes. So that the contributor will have an opportunity to find his error within a few minutes and can correct it and finish the merge request successfully with in few minutes.

I think the existing pre-commit-compile is just consuming meaninglessly "CPU time" in four (or more threads?) and not only the time of a developer but wasting electric energy of the country. (even Google Colab is doing the same mistake by, creating the user environment every time while running a code - of course it may be a 'cloud way'of doing things)

Google said each of the queries people raise will cause 0.2 grams of CO2 emissions. In that sense, the present pre-commit mechanism of ns3-dev will cause more CO2 emissions :-) :-) . 

I am writing an article/post on "My first experience on raising a simple merge request at GitLab".

I hope that it will be really useful for a beginner like me when trying to contribute something to ns-3.

It may save some time for a future contributor of ns-3.

 

Thanks for your support.

Charles Pandian,

And Finally ...

 

If I get any comments and reply to some of the above questions, then I will post them here.

 

I may post my experience on this review process on my submitted code in another Post

 

 

 

 

[Tom Henderson]

Charles, I have some replies inline below and I have also cc'ed Tommaso Pecorella who knows more about the CI behavior than I do.

On 12/19/21 12:29 AM, Charles Pandian wrote:

In this post, I want to share my first experience on 'Raising a simple merge request at GitLab' at https://gitlab.com/nsnam/ns-3-dev/-/merge_requests

You may find that specific request at - A Simple CircleMobilityModel (3D) for ns-3.

<snip>

My Questions are:
1. Why Doxygen is not failing on other ns-3 header files where there were no Doxygen comments at all?.

The CI system is set up to focus on changed Doxygen warnings for the files touched in the Merge Requests.

@Tommaso, is there a command that we can recommend to users to test Doxygen in the same way on their local machine before submitting a MR?

2. Why it is running the all the test of other models again and again at the end  of  all pre-commit-compile-* ?

If you change a comment in a header, the build system does not know whether you changed the code or the comment.  This will trigger a run of all of the tests (in the CI system) just to be safe.

3. Is this the 'normal' usual way we have to do things while merging after a "single line change in the comment line of the code?" 

    (I mean waiting for more than an hour for editing a single line of comment? :-) :-) :-) )

I do not know whether there are efficiencies that can be added to the CI system to speed it up.  I think the solution is generally to check things locally before submitting the merge request so that there are few surprises/failures from a MR.

4. Is there any other way to quickly do the  pre-commit-compile-*  and Doxygen compile only on the edited files?

For your information, I think, I am provided with a "shared pipeline" (I have no idea about it) since I am using a free account.

I am not sure whether there is any script to reproduce the CI checks on a local machine (perhaps just for the host's operating system, disregarding all of the Docker images).

- Tom

[Gabriel]

Skip the tests? There is a trick that could be used.

First zero out ccache statistics with `ccache -z` before building. Then build.

Before running the tests, get the ccache hit rate with `ccache -s`.

In GitHub I use `python3 -c "import re; print('::set-output name=cache_hit_rate::'+re.findall('.* (.*) %', '`ccache -s | grep "cache hit rate"`')[0])"` to export a pipeline output variable named cache_hit_rate.

If its a 100% hit rate, skip the tests. If it is not, run the tests. If the tests fail, do not save the ccache.

https://github.com/Gabrielcarvfer/NS3/blob/master/.github/workflows/per_commit.yml#L37-L54

Regarding the CI, gitlab-runner exec can probably help testing the pipelines locally (https://docs.gitlab.com/runner/commands/#gitlab-runner-exec)

But it has some limitations, so there are different alternatives such as 

https://pypi.org/project/gitlabci-local/

https://github.com/firecow/gitlab-ci-local

[Gabriel]

Just updating, I already had a patch for gitlab. :D

And the firecow gitlab-ci-local works pretty well. 

`/ns-3-dev$ sudo gitlab-ci-local --file ./utils/tests/gitlab-ci.yml per-commit-compile-release` downloads the docker image and runs the per-commit-compile-release task.

`/ns-3-dev$ sudo gitlab-ci-local --file ./utils/tests/gitlab-ci.yml --list` shows all available tasks.

[Charles Pandian]

Mr. Gabriel,

And the firecow gitlab-ci-local works pretty well. 
`/ns-3-dev$ sudo gitlab-ci-local --file ./utils/tests/gitlab-ci.yml per-commit-compile-release` downloads the docker image and runs the per-commit-compile-release task.

So, after (only) running the pre-commit-compile-release from a local docker image, will it automatically mark the status of the merge-request success at the remote GitLab IDE?

Or, do we have to resubmit the merge request from GitLab IDE again?

[Gabriel]

No, it won't affect the MR status, but will let you know if this part of the pipeline is working correctly. 

If you want to test only the documentation part, change the per-commit-compile-release to the documentation task. 

After fixing, sure, just force push/force push the fixed commit to the MR and wait for the pipeline to complete. 

[Charles Pandian]

No, it won't affect the MR status,

Thank you Mr. Gabriel.

I already made a little suggestion on re-engineering the pre-commit-compile-*Process (for GitLab IDE users like me):

The pre-commit compile is running as 4 separate processes.

So, if one made a "syntax error" in code, then these 4 pre-commit compile possesses will just waste CPU time in four threads and will end up with the same error after 2 hours.

If we allow only one thread (I mean only one pre-commit compile process at one time) at full speed, then it will end with the same error after a few minutes. So that the contributor will have an opportunity to find his error within a few minutes and can correct it and finish the merge request successfully with in few minutes. 

Of course, I can understand that, in a cloud environment, each merge request will initiate a new environment and download, install, compile everything and so that will consume lot of time. Even after correcting a single line and resuming that merge request, all the steps will be repeated.  Because, It will not have the previously compiled binaries (*.o files) in the new, fresh instance of the fork - so that it will compile everything again.

But what are the logical solutions for reducing pre-compile time(for GitLab IDE users like me)?

1). Saving the source files and the outputs of to compile (*.o files) in a  somewhat temporary/permanent location (at cloud environment) and merging it with the current fork and resuming the compile. It may be technically hard to implement in such a cloud environment(GitLab) - but it is theoretically possible. In that case, ./waf will not meaninglessly compile everything again and again.

2) a simple and possible solution will be: just rewriting the pre-compile script(which was invoked through GitLab IDE), in such a way to do things in a sequential manner (only one type of compile at a time)-  so that instead of running it in four different threads and letting the four threads end with the same error after 2 hours,  this sequential approach will minimize the compile-time, in case, the user code is having errors. Further, if it is allowed to run the four types of document compile process also in sequential/separately, then we need to wait for getting a Doxygen error (after the repeated successful compile of other three doc compiles)

Mr. Gabriel, I want your opinion on my second suggestion. (Will it be possible and will improve the user experience at GitLab IDE?)

Charles Pandian,

[Charles Pandian]

Mr. Gabriel,

Sorry for addressing the following doubt to you. I am asking this question because you talked about the word "MR status".

After the successful MR, I received an automated mail with the following content:

Pipeline has been fixed and #432474760 has passed!

 Project Charles Pandian / ns-3-dev
Branch master
Commit 7f024a5d in !821
Upload New File
Commit Author Charles Pandian

Of course, I can understand that after this, it will take a lot of time for getting feedbacks and to complete the review process. And will depend on the "importance" of the code and availability of the reviewers and their interest on that piece of code.

The question is :
As the "first" participant/maintainer my own project, the first participant(me), formally need to send an invitation to reviewers or necessary to add more participants after getting their acceptance? - so that the actual review will start?

"automated mail" only mentioned the success of the merge request - it does not say "what the first participant has to do after that".
For example, if the message would says like "SR was successful, you have to patiently wait for a  review process with respect to the availability of the reviewers", the first participant(me) need not get confused about "when the review start, who will stat the review"

I hope that you will understand this question from the perspective of a GitLab IDE user.

Charles Pandian,

--
Posting to this group should follow these guidelines https://www.nsnam.org/wiki/Ns-3-users-guidelines-for-posting
---
You received this message because you are subscribed to the Google Groups "ns-3-users" group.
To unsubscribe from this group and stop receiving emails from it, send an email to ns-3-users+...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/ns-3-users/1951e886-894b-4dd1-956f-3330ccf47bedn%40googlegroups.com.

[Charles Pandian]

And the firecow gitlab-ci-local works pretty well. 
`/ns-3-dev$ sudo gitlab-ci-local --file ./utils/tests/gitlab-ci.yml per-commit-compile-release` downloads the docker image and runs the per-commit-compile-release task.

For your information, the above will not be a suitable solution for one (like me) who is only having poor internet connection.
Because, downloading the docker image itself will consume additional time.

In addition to that, since it will not alter MR status, we have to do complete the MR at GitLab IDE only. So, the way in with we run/compile it at GitLab IDE should be optimized.

Am I correct?

Charles Pandian,

--

Posting to this group should follow these guidelines https://www.nsnam.org/wiki/Ns-3-users-guidelines-for-posting
---
You received this message because you are subscribed to the Google Groups "ns-3-users" group.
To unsubscribe from this group and stop receiving emails from it, send an email to ns-3-users+...@googlegroups.com.

To view this discussion on the web visit https://groups.google.com/d/msgid/ns-3-users/a12a29e8-8814-4e80-8a03-63f201694511n%40googlegroups.com.

[Gabriel]

1) You don't need to manually save the intermediate files. Ccache is doing that for you. 

2) This could be changed. Maybe Release->Doxygen->Debug|Optimized. 

But you should always test things locally first. That's why the gitlab-ci-local is pretty cool.

I've looked your MR pipeline and what happened makes sense to me. Skipping the tests with the ccache trick could save half an hour to an hour from those two hours.

[Charles Pandian]

2) This could be changed. Maybe Release->Doxygen->Debug|Optimized. 

Do you mean to set this at GitLab IDE-Project settings?  Where is that setting "Release->Doxygen->Debug|Optimized"?

Sorry for my ignorant question. (I could not find any such setting at GitLab IDE)

Charles Pandian,

--
Posting to this group should follow these guidelines https://www.nsnam.org/wiki/Ns-3-users-guidelines-for-posting
---
You received this message because you are subscribed to the Google Groups "ns-3-users" group.
To unsubscribe from this group and stop receiving emails from it, send an email to ns-3-users+...@googlegroups.com.

To view this discussion on the web visit https://groups.google.com/d/msgid/ns-3-users/459bc894-cce5-4cef-8e96-c4487c728185n%40googlegroups.com.

[Gabriel]

No, this needs to be changed in the utils/tests/gitlab-ci.yml

You would need to split up the build stage into two at least, one with just the Release type that comes first, and the other with the rest.

[Charles Pandian]

The following page 
https://www.nsnam.org/docs/manual/html/working-with-git.html
says :
After you push your branch to origin, you can follow the instructions here https://docs.gitlab.com/ee/gitlab-basics/add-merge-request.html to create a merge request. Please remember to add, as reviewer, at least one maintainer.


But I could not find an option to add a reviewer at GitLab IDE (of my project / merge request page).

Furthermore, the following link also not working.
https://docs.gitlab.com/ee/gitlab-basics/add-merge-request.html 

Charles Pandian,

[Tom Henderson]

On 12/20/21 10:43 AM, Charles Pandian wrote:

The following page 
https://www.nsnam.org/docs/manual/html/working-with-git.html
says :
After you push your branch to origin, you can follow the instructions here https://docs.gitlab.com/ee/gitlab-basics/add-merge-request.html to create a merge request. Please remember to add, as reviewer, at least one maintainer.


But I could not find an option to add a reviewer at GitLab IDE (of my project / merge request page).

Furthermore, the following link also not working.
https://docs.gitlab.com/ee/gitlab-basics/add-merge-request.html

Thanks, I have fixed the broken links and removed the recommendation to find a reviewer (a maintainer can take that step, if needed).

- Tom