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

update AlAs lattice parameter and Cij #165

Merged
merged 7 commits into from
Jul 4, 2023
Merged

Conversation

VeryBitter
Copy link
Contributor

Hi
Great library !
I found out the AlAs lattice parameter was inconsistent with our lab software database. I suggest to update it from this paper Appl. Phys. Lett. 66, 682–684 (1995): 5.66172 (instead of 5.6611). Our lab database indicates 5.66182 but I cannot find any reference yet.

Found a newer value for AlAs lattice parameter, closer to our lab's value
Update AlAs lattice parameter and Cij
@dkriegner
Copy link
Owner

thanks for the contribution. you are welcome to add yourself to the copyright statement at the top of the file.

@dkriegner
Copy link
Owner

/AzurePipelines run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@dkriegner
Copy link
Owner

The last commit uploaded does not at all fit the scope of this pull request and should be removed.

I also do not like the way its implemented since FitModel is derived from lmfit.Model it should follow its way of using keyword arguments. So the method keyword argument to the class constructor seems to be against this pattern. I understand that already now I break the compatibility with lmfit.Model in some positions but I think it should not get worse. So the method must be passed to FitModel.fit and in fact this is already possible via the lmfit_kws argument of FitModel.fit. If you see a way to improve the situation and keep or improve the compatibility with lmfit.Model suggestions are welcome.

@VeryBitter
Copy link
Contributor Author

VeryBitter commented Jul 3, 2023 via email

Deleting unwanted commit file
@VeryBitter VeryBitter closed this Jul 4, 2023
@VeryBitter VeryBitter reopened this Jul 4, 2023
@VeryBitter
Copy link
Contributor Author

Hi
I revert to the original models.py and fit.py files. I deeply apologize for all the mess.

@dkriegner dkriegner merged commit d8e88e0 into dkriegner:main Jul 4, 2023
@dkriegner
Copy link
Owner

dkriegner commented Jul 4, 2023

I merged the change of the AlAs lattice parameters. Thanks for the contribution!

I have recently changed this FitModel part myself since I realized that not all keyword arguments can be passed through. I guess it should now work (either with fit_kws or lmfit_kws), but I am not entirely happy with the way its done. If you see problems please open a github issue and we come up with a solution.

VeryBitter added a commit to VeryBitter/xrayutilities that referenced this pull request Jul 5, 2023
update AlAs lattice parameter and Cij (dkriegner#165)
dkriegner added a commit that referenced this pull request Jul 5, 2023
VeryBitter added a commit to VeryBitter/xrayutilities that referenced this pull request Jul 8, 2023
update Changelog with changes from dkriegner#165
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants