Skip to content

Agnanav - #34

Open
adamm2603 wants to merge 15 commits into
asgr:masterfrom
adamm2603:AGNanav
Open

adamm2603 wants to merge 15 commits into
asgr:masterfrom
adamm2603:AGNanav

Conversation

@adamm2603

Copy link
Copy Markdown
Contributor

Added in the option to used the angle averaged AGN SED instead of the chosen viewing angle SED for the calculation of attenuation of the AGN flux by the host galaxy ISM.

Default options produce the same AGN attenuation as current ProSpect version, but setting AGN_emission = 'angle_averaged' switches to this new mode (this can also be set to 'viewing_angle' or 'None' to use the chosen viewing angle AGN SED instead, or to turn off AGN attenuation completely). The complementary Fritz and SKIRTOR angle averaged data structures have also been added into ProSpectData and can be preloaded and input to ProSpectSED using the variable AGN_anav in the same way as the full Fritz and SKIRTOR data to avoid reloading from disk on each call.

Adds in ability to use angle averaged AGN SED for ISM attenuation of AGN
Adds in ability to use angle-averaged AGN SED for ISM attenuation
Adds in the ability to use angle-averaged AGN SED for ISM attenuation
Renamed AGN_emission to AGN_screen_model for clarity
Renamed AGN_emission to AGN_screen_model for clarity
Fixed plotting function to use correct starUnAtten ylim upper limit
Added 'None' option to remove AGN ISM attenuation
Added error messages to pre-loading AGN data
Added 'None' option to avoid AGN ISM attenuation.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

The new AGN modes fail to load required companion datasets, and unresolved regression-test gaps and plotting limits that clip luminous AGN spectra remain.

3 open issues
What changed in this PR

Adds angle-averaged AGN dust heating to ProSpect’s SED model while preserving viewing-angle-dependent direct emission and the existing default behavior.

Changes:

  • Adds selectable AGN screen modes and forwards them through the likelihood interface.
  • Introduces Fritz and SKIRTOR angle-averaged interpolators with optional preloaded templates.
  • Documents the new options and excludes wavelengths from the stellar luminosity plotting limit.
File Description
R/​ProSpect.R Implements AGN screen modes and likelihood argument forwarding.
R/​plots.R Uses stellar luminosity for the automatic upper plotting limit.
R/​AGN.R Adds angle-averaged template interpolation helpers.
man/​ProSpectSED.Rd Documents screen modes, templates, and likelihood options.
man/​AGNinterp.Rd Documents the new interpolators and clarifies output units.

Review effort: Balanced

Comment thread R/AGN.R Resolved
Comment thread R/ProSpect.R Resolved
Comment thread R/plots.R Resolved
Add AGN screen-mode regression tests
@asgr

asgr commented Nov 12, 2026

Copy link
Copy Markdown
Owner

@adamm2603 could you fix the objective issues highlighted (note there are suggested changes already). Also, I'm not keen to add 600 lines of new tests just for this new functionality, so either reduce to a very simple additional check/test, or remove entirely. Basically, that level of additional test is too much for the scale of the feature (since all tests add to the package test time).

@adamm2603

adamm2603 commented Nov 13, 2026 •

Copy link
Copy Markdown
Contributor Author

I believe these are resolved. I've removed the test, which I only added after the copilot recommendation anyway. The ProSpectData issue had to wait for the ProSpectData merge so I could update the version number, but that's fixed now.

The plot feature isn't anything to do with the new functionality, I just noticed that sometimes the y-axis was a bit off on the plots because it could use an upper ylim sometimes based on the maximum wavelength value rather than the maximum luminosity. I don't think the recommendation copilot suggested is likely to have much of an effect but I've added it.

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

Successfully merging this pull request may close these issues.

3 participants