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.

Sign up for GitHub

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

Jump to bottom

forward() should pick answer consistent with forward_solutions_table() and default decision function #315

Merged
merged 5 commits into from
Dec 20, 2023
Merged

forward() should pick answer consistent with forward_solutions_table() and default decision function #315

merged 5 commits into from
Dec 20, 2023

Conversation

prjemian
Copy link
Contributor

@prjemian prjemian added the bug label Dec 19, 2023
@prjemian prjemian added this to the v1.1 milestone Dec 19, 2023
@prjemian prjemian self-assigned this Dec 19, 2023
@prjemian prjemian mentioned this pull request Dec 19, 2023
Closed
@prjemian prjemian marked this pull request as ready for review December 19, 2023 06:46
@prjemian
Copy link
Contributor Author

This PR passes its own tests.

@prjemian prjemian mentioned this pull request Dec 19, 2023
Closed
3 tasks
@prjemian
Copy link
Contributor Author

@jwkim-anl : Looks like a simple fix.

@ambarb : Are you OK with this change?

padraic-shafer
padraic-shafer approved these changes Dec 19, 2023
View reviewed changes
Copy link
Contributor

@padraic-shafer padraic-shafer left a comment

Choose a reason for hiding this comment

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

This looks reasonable. The new test might have more rigid requirements than needed though.

hkl/tests/test_diffract.py Outdated Show resolved Hide resolved
@prjemian @padraic-shafer
Update hkl/tests/test_diffract.py
a7ed9ca 
LGTM

Co-authored-by: Padraic Shafer <[email protected]>
mrakitin
mrakitin approved these changes Dec 20, 2023
View reviewed changes
Copy link
Member

@mrakitin mrakitin left a comment

Choose a reason for hiding this comment

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

Looks reasonable to me.

@prjemian
Copy link
Contributor Author

@padraic-shafer @mrakitin Thanks!

@prjemian prjemian merged commit 7b57153 into main Dec 20, 2023
8 checks passed
@prjemian prjemian deleted the 303-v2-forward-solution branch December 20, 2023 03:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mrakitin mrakitin mrakitin approved these changes

@padraic-shafer padraic-shafer padraic-shafer approved these changes

Assignees

@prjemian prjemian

Labels
bug
Projects
None yet
Milestone
v1.1
Development

Successfully merging this pull request may close these issues.

BUG: forward() does not give same answer as first solution in forward_solutions_table()
3 participants
@prjemian @mrakitin @padraic-shafer