-
Notifications
You must be signed in to change notification settings - Fork 57
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
Optionally return the dislocation core fit error. #247
Conversation
@pgrigorev or @thomas-rocke would one of you be willing to take a look at this PR? |
@jameskermode, we have already discussed this PR with Ivan, but I will have a closer look on the changes and let you know here. |
The failing tests are due to an ASE change, which could be fixed here or in a separate PR. I think |
We've also got some numpy 2 issues. I'll open an issue about that. |
ah I see. Yes we should fix that. I guess the best way would be to check ase version on import to keep backward compatibility? |
I think the tests failing tests have been fixed now, @imaliyov could you merge it to your branch? |
@pgrigorev thank you, I have merged it to my branch. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Everything looks good, I am sure it is safe to merge!
Sorry it took so long, but after having a look on the changes I think it is safe to merge. (the changes are really minor) |
Thanks! |
Modified the
fit_core_position()
andfit_core_position_images()
functions of thedislocation.py
module.return_error
andreturn_errors
flags added. IfTrue
, the functions will return the error of thescipy.optimize.minimize
fit of the dislocation core position. This will require an additionalcost_function()
call. This probably can be avoided in the future if the error can be obtained directly from theminimize()
function.