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

freqs_init is never called and we use the same bands for photon generation as for ionization Jnu models #1084

Open
jhmatthews opened this issue Jun 1, 2024 · 2 comments

Comments

@jhmatthews
Copy link
Collaborator

Just making a note of this while I remember, but since PR #800 (see also issue #798) it seems freqs_init has not been being called in the code. Instead, around line 600 of bands.c, where we used to call it, we set the band boundaries to be the same as the photon generation ones.

This was not my understanding of how the code should work, so is this deliberate/intended?

I find it a bit concerning, because it could mean that the ionization results depend rather a lot on the banding choice, and I certainly was not aware of this when I was picking photon bands. It means the user also needs to be aware of this, and perhaps errors thrown if the bands are too coarse. As an example, if you choose CV banding you only have 4 bands in total to model the spectrum, which doesn't seem great to me.

@jhmatthews
Copy link
Collaborator Author

Even if this is the desired behaviour (which is fine), it strikes me it should be improved without much effort. For example, we know roughly how many photons are going to come out in a given band, so we could specify a parameter like DESIRED_PHOTONS_PER_BAND which would allow one to choose how many frequency bands to use -- perhaps with a minimum and maximum number. These bands would be divisions of the photon generation bands to avoid any of the issues to do with having a complete disconnect between the two.

@jhmatthews
Copy link
Collaborator Author

A related question that pops up here is that I don't know what happens if you illuminate something with, say, only an X-ray power-law, but you have significant reprocessing to lower frequencies. Either it keeps the banding you specified - in which case how on Earth does it satisfy the minimum fraction in that band? Or, it adjusts the minimum frequency of the bands, which would then be bad for your ionization calculation. One way or the other, this needs to be more robust.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant