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

Implement xtype other than tth #120

Merged
merged 6 commits into from
Nov 1, 2024
Merged

Conversation

yucongalicechen
Copy link
Collaborator

@yucongalicechen yucongalicechen commented Oct 31, 2024

closes #45

I implemented functions to allow corrections on grid other than tth. The edits include:

  • In the main module, replace all "tth" with args.xtype.
  • In the tools module, filter out invalid xtype or set args.xtype to "tth" or "q" based on inputs (this makes it easy to write the output files using the dump function).
  • In compute_cve function in functions.py, interpolate cve_do we get from the global tth to the input grid.

Question: I didn't see functions that allow transfer between tth and d, so when xtype=d do we simply put xtype=tth?
@sbillinge ready for some feedback.

@sbillinge
Copy link
Contributor

sbillinge commented Nov 1, 2024

Question: I didn't see functions that allow transfer between tth and d, so when xtype=d do we simply put xtype=tth? @sbillinge ready for some feedback.

@yucongalicechen this still needs to be implemented in diffraction_objects. Please could you make an issue if it is not already there? d = 2*pi/Q.

Copy link
Contributor

@sbillinge sbillinge 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 good. It may need a test for when an invalid input is provided for xtype. This can go in a separate test test_set_xtype_bad

@sbillinge
Copy link
Contributor

@yucongalicechen this code is very high quality. Great job! you have learned well!

assert actual_args.xtype == expected[0]


def test_set_xtype_bad():
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Here's the test function for bad cases :)

@yucongalicechen
Copy link
Collaborator Author

Question: I didn't see functions that allow transfer between tth and d, so when xtype=d do we simply put xtype=tth? @sbillinge ready for some feedback.

@yucongalicechen this still needs to be implemented in diffraction_objects. Please could you make an issue if it is not already there? d = 2*pi/Q.

@sbillinge
Sounds good, I’ll add an issue in diffpy.utils.
A related question is that the dump function only writes tth and q - do we want to add q as well?
Another related question - I think we would like to have a function in diffpy.utils that interpolate DOs (as described in #94). Is this the issue here (diffpy/diffpy.utils#48), or shall I open another issue for this?

@sbillinge
Copy link
Contributor

Question: I didn't see functions that allow transfer between tth and d, so when xtype=d do we simply put xtype=tth? @sbillinge ready for some feedback.

@yucongalicechen this still needs to be implemented in diffraction_objects. Please could you make an issue if it is not already there? d = 2*pi/Q.

@sbillinge Sounds good, I’ll add an issue in diffpy.utils. A related question is that the dump function only writes tth and q - do we want to add q as well? Another related question - I think we would like to have a function in diffpy.utils that interpolate DOs (as described in #94). Is this the issue here (diffpy/diffpy.utils#48), or shall I open another issue for this?

yes, an issue for a user-supplied master-grid is already there i think. Yes, dump dumping d might be nice

@sbillinge sbillinge merged commit 651248d into diffpy:main Nov 1, 2024
3 checks passed
@yucongalicechen yucongalicechen deleted the xtype branch November 1, 2024 12:24
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.

implement other x-axes
2 participants