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

Velodiff, take 2 #317

Merged
merged 4 commits into from
Aug 8, 2024
Merged

Velodiff, take 2 #317

merged 4 commits into from
Aug 8, 2024

Conversation

sbailey
Copy link
Collaborator

@sbailey sbailey commented Jul 26, 2024

This PR superseded PR #315 from @abrodze with description "changes to address issues with max_velo_diff variable explained in #312. Seems to work as intended, but it is still in test mode." This has the same changes, but has a cleaner difference history since I messed up her previous branch.

@sbailey
Copy link
Collaborator Author

sbailey commented Jul 26, 2024

Shoot; mis-communication; looks like @abrodze made another change on velodiff while I was making this. Will go chat in person, apologies for the github noise...

@coveralls
Copy link

coveralls commented Jul 26, 2024

Coverage Status

coverage: 38.584% (+0.03%) from 38.554%
when pulling ebf0ee2 on velodiff2
into 0428a3a on main.

@abrodze
Copy link
Member

abrodze commented Jul 30, 2024

Testing the most recent commit on dark time from healpix 128, 4511, 8975, 9020, and 20410 yields the following zwarn changes:

TARGETID BEST SPEC/SUB BEST Z 2nd SPEC/SUB 2nd Z ZWARN Jura ZWARN velodiff
39627918145490404 QSO-LOZ 1.570988329023113 QSO-HIZ 1.5722801626648066 4 0
39628295049841654 QSO-LOZ 1.5423072585152233 QSO-HIZ 1.5355944974929774 4 0
39628289404308576 QSO-HIZ 1.550410268576145 QSO-LOZ 1.5516679257428017 4 0
39628289412697609 QSO-LOZ 1.4023193160330976 QSO-HIZ 1.4083295493824548 4 0
39628352079794080 QSO-LOZ 1.5791245239679095 QSO-HIZ 1.5782587641246637 4 0
39628439069657952 QSO-LOZ 1.492384745922154 QSO-HIZ 1.491430594388815 4 0

I am happy to sign off on this PR.

@abrodze abrodze requested a review from akremin August 8, 2024 00:51
Copy link
Member

@akremin akremin left a comment

Choose a reason for hiding this comment

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

Thank you the nicely commented changes. I think the code looks good and I'll take your small healpix test as validation. In preparation for Kibo we will perform a larger test run, which we should re-run your tests on to verify this again.

@akremin akremin merged commit fd8f666 into main Aug 8, 2024
12 checks passed
@akremin akremin deleted the velodiff2 branch August 8, 2024 18:48
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.

4 participants