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

fix for #168 #169

Merged
merged 2 commits into from
Nov 14, 2014
Merged

fix for #168 #169

merged 2 commits into from
Nov 14, 2014

Conversation

mklauser
Copy link
Contributor

The lines of code that caused the error in tardis/cmontecarlo.c are removed. Since these lines are only used for debugging, there should be no side effect on other parts of the code.

@orbitfold
Copy link
Contributor

How about checking if the array index is valid and only execute those lines then?
Išsiųsta BlackBerry® paštu iš Bitės

-----Original Message-----
From: Michael notifications@github.com
Date: Wed, 13 Aug 2014 07:19:17
To: tardis-sn/tardistardis@noreply.github.com
Reply-To: tardis-sn/tardis reply@reply.github.com
Subject: [tardis] fix for #168 (#169)

The lines of code that caused the error in tardis/cmontecarlo.c are removed. Since these lines are only used for debugging, there should be no side effect on other parts of the code.
You can merge this Pull Request by running:

git pull https://github.com/mklauser/tardis fix_168

Or you can view, comment on it, or merge it online at:

#169

-- Commit Summary --

-- File Changes --

M tardis/cmontecarlo.c (4)

-- Patch Links --

https://github.com/tardis-sn/tardis/pull/169.patch
https://github.com/tardis-sn/tardis/pull/169.diff


Reply to this email directly or view it on GitHub:
#169

@mklauser
Copy link
Contributor Author

Checking the array index is also a way to go. However, in my opinion, this is more expensive, and (I think) we don't need this value in compute_distance2line at all.

@orbitfold
Copy link
Contributor

Maybe move it to the error reporting if clause.
Išsiųsta BlackBerry® paštu iš Bitės

-----Original Message-----
From: Michael notifications@github.com
Date: Wed, 13 Aug 2014 07:42:00
To: tardis-sn/tardistardis@noreply.github.com
Reply-To: tardis-sn/tardis reply@reply.github.com
Cc: Vytautas Jancauskasunaudio@gmail.com
Subject: Re: [tardis] fix for #168 (#169)

Checking the array index is also a way to go. However, in my opinion, this is more expensive, and (I think) we don't need this value here at all.


Reply to this email directly or view it on GitHub:
#169 (comment)

@wkerzendorf
Copy link
Member

Great @mklauser! I agree with @orbitfold to movie it to the error reporting if clause. I think it would also be great to get some tests going there, which should be relatively easy.

@wkerzendorf
Copy link
Member

@mklauser can you move the offending lines to the error clause and rebase this PR (to master that was just fixed). Then we can merge this.

@wkerzendorf
Copy link
Member

@mklauser What's going on with this? Can you fix it so we can merge this.

@@ -132,8 +130,6 @@ inline tardis_error_t compute_distance2line(rpacket_t *packet, storage_model_t *
fprintf(stderr, "comov_nu = %f\n", comov_nu);
fprintf(stderr, "nu_line = %f\n", nu_line);
fprintf(stderr, "(comov_nu - nu_line) / nu_line = %f\n", (comov_nu - nu_line) / nu_line);
fprintf(stderr, "last_line = %f\n", last_line);
Copy link
Member

Choose a reason for hiding this comment

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

move line 123-124 -> 135, 136

@wkerzendorf
Copy link
Member

@mklauser looks good - merging

wkerzendorf added a commit that referenced this pull request Nov 14, 2014
@wkerzendorf wkerzendorf merged commit 888e3e6 into tardis-sn:master Nov 14, 2014
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.

3 participants