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

Add support for anomalous data to DataSet.to_reciprocalgrid() #65

Open
JBGreisman opened this issue Jun 9, 2021 · 5 comments
Open

Add support for anomalous data to DataSet.to_reciprocalgrid() #65

JBGreisman opened this issue Jun 9, 2021 · 5 comments
Labels
API Interface Decisions enhancement Improvement to existing feature

Comments

@JBGreisman
Copy link
Member

DataSet.to_reciprocalgrid() does not currently enable data to differ between Friedel halves of reciprocal space. This functionality would be useful for things like generating maps from underlying anomalous data. As far as API, this could be implemented with an anomalous=True|False argument. However, it may involve a decision such as how to specify the anomalous columns labels.

Internally, if such a function were called with a two-column anomalous DataSet, it would be possible to implement this method using a call to stack_anomalous() in the place of expand_anomalous(). Implicit in this is that the (+)/(-)-suffixed columns would be renamed to drop the Friedel specifications.

@JBGreisman JBGreisman added API Interface Decisions enhancement Improvement to existing feature labels Jun 9, 2021
@minhuanli
Copy link
Contributor

Hey Jack, I am wondering what is the current situation of the issue (Maybe this is not an issue anymore)?
If I have one-column anomalous data (similar to that after calling stack_anomalous), is there anything wrong if I call to_reciprocal_grid?

@minhuanli
Copy link
Contributor

If I understand correctly, say I have a stacked one-column anomalous data, the only difference would be I have to skip the line p1 = p1.expand_anomalous() in to_reciprocal_grid function?

@JBGreisman
Copy link
Member Author

Yes -- this is currently unsupported because of the line you mentioned. If one-column anomalous data is provided, it should be sufficient to just skip the call the p1.expand_anomalous(), but I haven't actually tested that.

For 2-column anomalous data, a few more changes will be needed because a call to stack_anomalous() will be needed. We would also need to make some API decisions about how the two columns are specified (e.g. tuple of column labels)

@minhuanli
Copy link
Contributor

Yes, that makes sense.

How about taking another argument like anomalous=False? And when set to True, whether to callstack_anomalous or skip that line can be determined by the number of columns in the key?

I am happy to create a PR for this if you think above is a good way.

@JBGreisman
Copy link
Member Author

I think we can make the interface work intuitively without the need for anomalous=[True|False]. I think the following should work, but let me know if you think I'm missing something:

  • If key is a single string, we check whether all HKLs are in the + ASU. If they are, we call p1.expand_anomalous(). If the HKLs are in +/- ASU we skip the call to p1.expand_anomalous(). This should support non-anomalous data, or single-column anomalous.
  • If the key is a pair of strings, we assume that we are operating in two-column anomalous mode. We call stack_anomalous(), and then treat it as one-column anomalous data as above.

Implicitly, there are two checks that will automatically happen:

  1. stack_anomalous() will confirm that the two columns have the same dtype for us when operating in two-column mode.
  2. expand_to_p1() will ensure that we are operating on a merged DataSet. This function only makes sense for merged data with unique Miller indices, because otherwise the order of the data will matter (the last entry for each Miller index will overwrite the preceding entries in the reciprocal grid).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
API Interface Decisions enhancement Improvement to existing feature
Projects
None yet
Development

No branches or pull requests

2 participants