sirocco-rt / sirocco

This is the repository for Sirocco, the radiative transfer code used to model winds in AGN and other systems
GNU General Public License v3.0
30 stars 24 forks source link

Proposed rename of line transfer modes #1095

Open jhmatthews opened 3 months ago

jhmatthews commented 3 months ago

We currently have these RT modes. I propose renaming four of them to match the release paper better -- here are some ideas.

jhmatthews commented 3 months ago

The basic point is that thermal trapping only describes the anisotropic mode and even then isn't that descriptive. But these modes are used to control how we enforce much higher level things like packet indivisibility / radiative eq. / energy conservation, and the names should reflect that. We would keep synonyms to retain backwards compatibility.

kslong commented 3 months ago

I have wondered if we should not just have two modes, unless extra diagnostics are invoked, e.g basic and macro. I agree basic is not a good name

jhmatthews commented 2 months ago

I think we've settled on classic and macro for these two modes. We should rename the "classic" non relativistic mode to something else (e.g. nonrel, classical, no-sr) to avoid any confusion.

jhmatthews commented 2 months ago

@kslong is there any way of preserving backwards compatibility here? i.e. I know we have synonyms for keywords, but do we also have a way of doing synonyms for answers to the keywords? Otherwise it means that changing a keyword means one can't run, e.g., a simulation grid easily with new versions of the code.

kslong commented 2 months ago

No, we don't have anything in place for synonyms of entries to keywords.

In principle, we could create something analogous to what we do for the keywords, e.g simply created a list of synonyms, and if an entry does not match one of the keywords, search this list for the current version, and if it finds it use that version.

I assume we are only talking about "rdchoice", so if that is the case we might be able to insert something there that is specific to each keyword, where we need the capability.

In python, one could do this with a dictionary, where one had multiple names, and more than one name pointed to the same outcome. There are ways to create the equivalent of dictionaries using structures, but rdchoice is not written around the idea.

Note that I am a little concerned about the "use case". Do we want to run "old style" grids with the new code. This would leave material that is confusing for "newbies" in the repository. We will not be able to use "new style" grids with the old code in any event. It might be better to simple fix the python scripts that create the grid.

(As a kind of aside, I do all my grid creation today inside Jupyter scripts; which makes creating versions of .pf files straight forward. One could easily imagine putting options in such scripts to handle this kind of issue, especially if there are not too many grids we are concerned about.)

For the capability we have namely for the keywords themselves, running the program renames the keyword to the new value, and so if you one a new version of the code once, then you get out a pf file with the current keyword names. When I wrote the synonym for keywords routine, my assumption was that it could be gradually phased out for this reason.

jhmatthews commented 2 months ago

I wouldn't say we exactly "want" to run old style grids, but I feel like a user should, ideally, be able to run parameter files from, e.g., our public simulation grids, with both the new and old version of the code. Same would apply to, e.g., the release models in future if we change things. I realise it won't be possible to run new grids with old versions though without a conversion.

I assume we are only talking about "rdchoice", so if that is the case we might be able to insert something there that is specific to each keyword, where we need the capability.

Yes, I think so, and I'd also wondered about that.

Another simple way around it is just to add both options to the rdchoice string, right? - the only problem there is that it prints that string to the new output file. But perhaps a new function or a mode in rdchoice that takes two "answer" strings as input would be the way forward there. It's hacky but it would work.

jhmatthews commented 2 months ago

What I mean is one would have a new rdchoice command like this:

user_line_mode = 
  rdchoice_hidden_modes 
("Line_transfer(pure_abs,pure_scat,sing_scat,escape_prob,thermal_trapping,classic,macro,macro_atoms_escape_prob,macro_atoms_thermal_trapping)",
     "0,1,2,3,5,5,7,6,7", answer,
     "Line_transfer(classic,macro)");

the last string being the string you would write out to .out.pf

kslong commented 2 months ago

Yes, I I agree I've thought about that options similar to this as well, including adding an extra argument for hidden modes, e.g

rdchoice("LIne_transfer",current_choices="a,b,c",outdated_choices="a_old,b_old,"c_old"],values="0,1,2,0,1,2")

My main reluctance is about the grids. This means you intend to keep grid running programs in the old formats, and I think this will be confusing over time.

I've also wondered about using this, or something similar to make it easier to change the options for testing. For example, in the example you have presented should we really offer users only a couple of options, and activate others for debugging, etc.

jhmatthews commented 2 months ago

It's ok, I really don't intend to keep running anything in the old formats (and I never do this). It's just about retaining (some) backwards compatibility, and I'm really talking about specifically public data that is already out there or will be out there and is associated with a specific paper or project. I don't wish to update or maintain those, and yes if I ran grids I would just script them based on an up to date parameter file.