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

Merge with pyradex? #5

Open
keflavich opened this issue Feb 9, 2022 · 2 comments
Open

Merge with pyradex? #5

keflavich opened this issue Feb 9, 2022 · 2 comments

Comments

@keflavich
Copy link

Since you're also using f2py to provide a python frontend to RADEX, are you interested in using pyradex for this?

https://github.com/keflavich/pyradex

It's the same thing - f2py-wrapped radex - but with some additional frontend using astropy units.

Whether you use it or not, I'd be interested in how you are keeping up with fortran versions - I've had several build problems with recent versions of numpy.

@jonholdship
Copy link
Collaborator

Thanks for raising this issue! It does seem like wasted effort for multiple people in the community to be maintaining different RADEX implementations. It would be good to merge if we can align the codes.

The reason I wrote spectralradex is that, by modifying the RADEX source code, I've made a version without COMMON blocks to allow parallelization. If you check src/radex_src/, you'll find a modern fortran version of RADEX. Even if we don't fully merge, you may want to take that for pyradex?

I don't have any issues with numpy, which may be the modern fortran working better with f2py. Spectralradex is also much faster than pyradex according to this nice BSc thesis (Table 2). I wonder if the updated fortran is also the reason for that?

So from my side, I think the modified RADEX is the main thing spectralradex can bring to pyradex. I'll have a proper look at pyradex over the next few days to see how the codes differ and gather some thoughts on how/whether we should merge. My email is holdship@strw.leidenuniv.nl if you'd like to organize a zoom about this.

@keflavich
Copy link
Author

The reason I wrote spectralradex is that, by modifying the RADEX source code, I've made a version without COMMON blocks to allow parallelization. If you check src/radex_src/, you'll find a modern fortran version of RADEX. Even if we don't fully merge, you may want to take that for pyradex?

Ah, that's super useful! The common blocks were a huge pain. I just made minimal changes to the source needed to get it to compile

I don't have any issues with numpy, which may be the modern fortran working better with f2py. Spectralradex is also much faster than pyradex according to this nice BSc thesis (Table 2). I wonder if the updated fortran is also the reason for that?

That's super helpful. That's a very impressive write up for a BSc. And, yeah, I would not be surprised if the modernized fortran plays nicer with f2py.

So from my side, I think the modified RADEX is the main thing spectralradex can bring to pyradex. I'll have a proper look at pyradex over the next few days to see how the codes differ and gather some thoughts on how/whether we should merge. My email is holdship@strw.leidenuniv.nl if you'd like to organize a zoom about this.

Yeah, that sounds right. It might be a few days' work, or a bachelors project, to get the new version merged in to pyradex.

I'll be happy to get on the line, though I'm definitely not going to have the time to put in real coding effort in the next few months.

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

No branches or pull requests

2 participants