-
Notifications
You must be signed in to change notification settings - Fork 3
Changes from thesis writing #220
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
Merged
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Since the comparison is to zero, it must be an atol, but it should scale with radius on the disc, since it's harder to get the same accuracy at greater distances.
Add a few more steps to the adaptive refine solve to try and smoothen out the disc plane.
This lets the block-based refinement strategy also use periodic blocks in the phi coordinate, by allowing them to wrap round. I haven't rigorously tested this to make sure it's doing what it is supposed to (thesis stress), but the results seem better.
For pre-computation, added a few new binning functions that can be used to bin up the timing and redshift data, as well as the emissivities without including any redshift factors.
Can now read out the status code for each photon traced in the adaptive sky, which makes it easier to calculate things like the reflection fractions. This commit will need revisiting at a later date, as I put it together quite hackily.
Closed
This should be ammended with the use of `@spawn` and tasks in the future, but I want to merge the `thesis` branch before addressing that. Julia 1.12 has interactive threads, so the thread IDs no longer behave as expected. This was always an anti-pattern I suppose, so should switch to the better semantics.
e88aae4
to
10a11ea
Compare
There have been a few definition changes that have affected some of the regression test values, such as changes to how the geometric grid is defined. The changes is flux in the 2D transfer functions is slightly more alarming, but I've checked that the documentation examples can be reproduced, as well as the figures from the paper, so I think it might just be some noise artifact. Those tests should probably be updated. Also removed the Voronoi tessellation tests, since that part of the code is no longer used and the underlying library keeps changing its primitives. All dependencies on that library should be removed in the future.
The new algorithm is extremely sensitive to having the right beta_0 offset parameter, and if it's too big, will take along time to fail, spurting "exceeded maxiters" warnings. That is obviously not ideal and should be examined in more detail in the future, but my priority at the moment is to get these commits merged in. Another aspect that was driving the CTF solving time up was running the root-solver twice, once over the datum plane and again against the actual thick disc. This commit removes that, and just checks if the impact parameters from the datum plane actually hit close enough on the thick disc. Updated the regression test values.
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #220 +/- ##
===========================================
- Coverage 68.75% 43.61% -25.15%
===========================================
Files 66 75 +9
Lines 2810 4228 +1418
===========================================
- Hits 1932 1844 -88
- Misses 878 2384 +1506 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
No description provided.