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.

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

Montecarlo progress #485

Merged
merged 2 commits into from
Feb 12, 2016
Merged

Conversation

yeganer
Copy link
Contributor

@yeganer yeganer commented Feb 12, 2016

This is a quick implementation that prints and updates a status line
containing the how many packets are already finished, how many total
packets are there and the percentage of finished packets.

The format of the statusline can be changed by the gobal macro
STATUS_FORMAT.

The implmentation is exact when running without OMP and an approximation
when activating OMP. This is not garanteed to work bugfree in case the
sheduler for the for-loop changes.
The alternative would be to have a shared variable but that could lead
to a minor performance reduction.

@unoebauer
Copy link
Contributor

I'm not sure if we want to have the indentation fix clog up the diff... Also, do we actually have clear C-style conventions, @wkerzendorf, @orbitfold?

@wkerzendorf
Copy link
Member

yes we do have conventions - @orbitfold ?

@yeganer
Copy link
Contributor Author

yeganer commented Feb 12, 2016

Sure, i can delete the indentation fix and make a new PR for it. There were mixed tabs and spaces so i just did :retab and gg=G in vim. I'll look into the conventions and adjust the indentation and remove it from this PR.

@yeganer yeganer force-pushed the montecarlo_progress branch from 9958631 to 3c32dd8 Compare February 12, 2016 11:34
@orbitfold
Copy link
Contributor

We do. We have agreed to follow gnu conventions for C code.
On 12 Feb 2016 13:18, "Wolfgang Kerzendorf" [email protected]
wrote:

yes we do have conventions - @orbitfold ?


Reply to this email directly or view it on GitHub.

@orbitfold
Copy link
Contributor

Also yes. For formatting/conventions/bookkeeping purposes please do a
separate OR.
On 12 Feb 2016 14:02, "Vytautas Jancauskas" [email protected] wrote:

We do. We have agreed to follow gnu conventions for C code.
On 12 Feb 2016 13:18, "Wolfgang Kerzendorf" [email protected]
wrote:

yes we do have conventions - @orbitfold ?


Reply to this email directly or view it on GitHub.

@unoebauer
Copy link
Contributor

Particularly in light of GSoC 2016, but also in general, we should prominently mention and link these conventions.

This is a quick implementation that prints and updates a status line
containing the how many packets are already finished, how many total
packets are there and the percentage of finished packets.

The format of the statusline can be changed by the gobal macro
STATUS_FORMAT.

The implmentation is exact when running without OMP and an approximation
when activating OMP. This is not garanteed to work bugfree in case the
sheduler for the for-loop changes.
The alternative would be to have a shared variable but that could lead
to a minor performance reduction.

Rename progress tracking variable in cmontecarlo.c

Change STATUS_FORMAT to look nicer
@yeganer yeganer force-pushed the montecarlo_progress branch from 41a8277 to c1a316b Compare February 12, 2016 13:15
@yeganer
Copy link
Contributor Author

yeganer commented Feb 12, 2016

@orbitfold @unoebauer @wkerzendorf I'm finished with the implementation. If you have anything to add to the feature, i'd be happy to implement it otherwise please have a quick look and then merge.

@@ -829,41 +832,56 @@ montecarlo_main_loop(storage_model_t * storage, int64_t virtual_packet_flag, int
#ifdef WITHOPENMP
omp_set_dynamic(0);
omp_set_num_threads(nthreads);
#pragma omp parallel

#pragma omp parallel firstprivate(finished_packets)
Copy link
Member

Choose a reason for hiding this comment

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

why is this change in there? I don't think that has anything to do with the logging, right?

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 made finished_packets ( my variable to count ) firstprivate so it won't be uninitialized in the threads. I made it private so it won't impact performance.

@unoebauer
Copy link
Contributor

Tested it with tardis_example and it works both with OMP on and off.

@unoebauer
Copy link
Contributor

@wkerzendorf, @orbitfold - happy with this? I'd merge

wkerzendorf added a commit that referenced this pull request Feb 12, 2016
@wkerzendorf wkerzendorf merged commit 642c781 into tardis-sn:master Feb 12, 2016
@yeganer yeganer deleted the montecarlo_progress branch March 30, 2016 13:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants