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 ring pattern generator #117

Merged
merged 4 commits into from
Dec 23, 2020
Merged

Conversation

AgBorrelli
Copy link
Contributor

Add ring pattern calculator to address #88

@dnjohnstone dnjohnstone added this to the v0.4.0 milestone Sep 5, 2020
@dnjohnstone dnjohnstone added the enhancement New feature, request, or improvement label Sep 5, 2020
@pc494
Copy link
Member

pc494 commented Sep 5, 2020

Hi @AgBorrelli, great that you've got going on this. When you want some feedback feel free to request a review from me.

@dnjohnstone
Copy link
Member

Thanks for chipping in @pc494 - I think the plan now is to use get_profile_data to create a ProfileSimulation (which is all in the code) and then add a method to ProfileSimulation like get_ring_pattern using the code introduced so far in this PR to get a numpy array of the ring pattern.

Does that make sense?

I think it highlights also that we might want to consider the name Profile in general... but I'm not sure if 1D or whatever would be better?

@dnjohnstone dnjohnstone linked an issue Sep 5, 2020 that may be closed by this pull request
@pc494
Copy link
Member

pc494 commented Sep 5, 2020

Sounds great, given the potential rename I think 0.4 is the correct label.

@dnjohnstone dnjohnstone mentioned this pull request Sep 9, 2020
3 tasks
@dnjohnstone
Copy link
Member

Is this still active @AgBorrelli - do you need any input here?

@AgBorrelli
Copy link
Contributor Author

Is this still active @AgBorrelli - do you need any input here?

Yes still active, I had a few corrections needed to MPhil thesis and DC was very slow. Haven’t been back on to this until I will start again on Monday. Got a lot of reading to do and this is also definitely on upcoming list. I had been doing tutorials to understand what a class is first, etc. Thanks

@pc494 pc494 mentioned this pull request Dec 5, 2020
6 tasks
@pc494 pc494 removed this from the v0.4.0 milestone Dec 17, 2020
@dnjohnstone
Copy link
Member

dnjohnstone commented Dec 23, 2020

@pc494 - how would you feel about a merge-as-is on this for diffsims-0.4.0 given it doesn't seem to be moving much and I think that would allow the corresponding code to be removed from pyxem at least tidying that part up?

@pc494
Copy link
Member

pc494 commented Dec 23, 2020

I had thought about this, but then felt like I had too many pyxem bits on my plate to want to do it right away, that load seems to have eased since then, so I'm in favour.

I will merge this PR into master, and then merge master into #143 (which is yet to go in) and then update the Changelog + API import there if that sounds good to you?

@dnjohnstone
Copy link
Member

yep sounds good

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature, request, or improvement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

implement RingPatternGenerator
3 participants