cholla-hydro / cholla

A GPU-based hydro code
https://github.com/cholla-hydro/cholla/wiki
MIT License
60 stars 32 forks source link

Add a check to indicate that MHD does not support projections #365

Closed bcaddy closed 5 months ago

evaneschneider commented 5 months ago

I don't really understand this. Why can't projections be output when MHD is turned on?

bcaddy commented 5 months ago

I haven't added support for it to the magnetic fields so if you run an MHD sim with it you'll just get the hydro fields which you probably don't expect. I'm happy to convert this to a warning instead of an error.

I can put adding support for projections to my to-do list if you want.

evaneschneider commented 5 months ago

I definitely don't think it should be an error if the code works, since it's likely that you will still want projections of the hydro variables when running with MHD. I'm not aware of a typical method for projections of the B-field itself, so my guess is actually that most people will still just be expecting projections of the hydro variables. (Cholla only supports density and temperature projections right now, anyway, which are the most common.) So, I don't even think a warning is necessary. If anything, I'd put a warning for slice outputs (but maybe those do support B-fields?).

bcaddy commented 5 months ago

I'll switch it to a warning. I'd like to have some kind of notification baked in about it so folks don't expect something they don't get and then get confused about it.

Slices support MHD using cell centered magnetic fields.

bcaddy commented 5 months ago

wait, don't the temperature calculations need some tweaks to support MHD?

evaneschneider commented 5 months ago

Yes, good point. So probably the clearest thing is to specify in the warning that the temperature projections don't take into account the magnetic pressure. (I still don't think this is that big a deal, since they're density weighted and unlikely to be used as anything other than a qualitative output.)

bcaddy commented 5 months ago

Would it be that hard to add the magnetic pressure to the temperature calculations? I assume it's just a couple of lines

evaneschneider commented 5 months ago

I think it would be extremely easy. I assumed this PR meant you didn't feel like it!

bcaddy commented 5 months ago

I did not dig that deep, I just noticed when writing the MHD wiki page that projections weren't supported and we should probably error/warn on that. I'll look into it tomorrow