nikolasibalic / ARC-Alkali-Rydberg-Calculator

Object-oriented Python library for computation of properties of highly-excited Rydbeg states of alkali and divalent atoms.
https://atomcalc.org
BSD 3-Clause "New" or "Revised" License
86 stars 72 forks source link

AC Stark Maps #138

Closed dihm closed 1 year ago

dihm commented 1 year ago

Nikola,

I've finally gotten around to polishing up some code I wrote a while back to calculate Stark maps due to arbitrary RF fields using a Floquet method that leverages ARC (see Meyer et al J Phys B 2019). I presently have it packed up as a stand-alone package for our lab's internal use but figured it would probably be better to 1) open-source it for others to use as we've found it helpful beyond that paper and 2) incorporate it into ARC directly so it is easier for others to find an use.

Before making a PR, I just wanted to confirm what you would prefer. First, are you interested in incorporating the functionality? Second, where would it go in the codebase/docs? Present code is three classes in three source files with full docstrings and two example jupyter notebooks with moderate levels of explanatory comments.

nikolasibalic commented 1 year ago

Hi David @dihm ,

This sounds like an amazing code contribution. Of course, I would be very glad to help with integration.

For functionality, it would make sense (as I understand) to go under Single Atom Calculations. Then it would be exposed on corresponding docs page we just need to add reference here.

For examples, it would make sense to display them with other .ipynb notebooks here, by adding notebooks here, and linking them in docs.

I would be happy to help with this, if you open branch and start PR.

Aside from this, we should revise the way citations are done. Up to know there was this note, with larger contributions in advanced. There it was clearly indicating who should be cited. But I think that widely useful functions, like you suggest, are much better exposed in correct place in core package. To start, you should add note in docsstring that your paper should be cited. And then I think good solution would be maybe that I develop some function that tracks which functions have been called from ARC package, and then have arc.export_citations() that would say basically, you used this n times, thus you should cite A. That would help keeping track of credits for future. What you think? Do you have better suggestions?

Looking forward to seeing this!

dihm commented 1 year ago

Awesome! I'll get a PR together next week (I'm actually on vacation at the moment). I tried to make sure everything would be ready to copy-paste so it won't take long once I get to it.

As for citations, I agree that something more flexible than the advanced submodules would be a good feature. Your suggestion sounds reasonable to me. My only thought would be that it would be nice to ensure it doesn't cause some ridiculous overhead in calculation times (probably not hard) and also doesn't require manually adding a method call to every single function of ARC (maybe unavoidable).

nikolasibalic commented 1 year ago

Thank you David @dihm for this great contribution!

I did some refactoring 9984f061e0955ea6cb973eed1f1ed429fddc323c to rename few things:

I also included link to newly added notebook to Getting started page and added getCitationForARC() function that can be called at the end of ARC use, and will pull citations according to the used parts of the code.

After that I run all ARC code through black formatter and flake8 linter, and now all imports are explicit (there was a lot of wildcard imports before), so if you notice some used method is not exposed anymore, please an issue.

This is available now in version 3.3.0 of ARC.

dihm commented 1 year ago

Thanks Nikola for getting this merged in. Those renames are all very reasonable.

I'm glad I've been able to contribute back to this excellent project.