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

Remove rpacket.recently_crossed_boundary flag #533

Merged
merged 1 commit into from
Apr 1, 2016

Conversation

yeganer
Copy link
Contributor

@yeganer yeganer commented Mar 31, 2016

Using the new distance2boundary calculation, we don't need recently_crossed_boundary flag, so it is
removed from all occurences.
This flag covered only outward propagating packets which have mu>0
anyway and result in the same calculation. This commit removes that
overhead and improves readability of the code.

This is in line with the discussion in Issue #517 .
If we are not 100% sure that there is no negative side effect, I'd suggest waiting for the improved C tests. Adding simple counters showed that recently_crossed_boundary == 1 implies mu > 0 for all testes setups (w7, abn_tom, example).

Using the new distance2boundary calculation, we don't need recently_crossed_boundary flag, so it is
removed from all occurences.
This flag covered only outward propagating packets which have mu>0
anyway and result in the same calculation. This commit removes that
overhead and improves readability of the code.
@ssim
Copy link
Contributor

ssim commented Mar 31, 2016

This looks good to me - happy to sign off is someone else is too!

@wkerzendorf wkerzendorf merged commit f036456 into tardis-sn:master Apr 1, 2016
kdexd pushed a commit to kdexd/tardis that referenced this pull request Apr 5, 2016
- tardis-sn#533 removed recently_crossed_boundary flag and was
  merged. The corresponding declaration was removed
  from the python counterpart struct of RPacket for
  restoring builds on travis.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants