Cosmic Ray parameter bug

[Andrew Emerick]

Hi All,

I've noticed a bug, but have not put time to tracking it down.

If one accidentally sets "CRDiffusion = 1" while "CRModel = 0", you get pretty whacky results (in my case, a galaxy that pretty immediately blows up). I doubt this was intentional but am not sure if it would be trivial to fix in the code somewhere (maybe just an if statement to set CRDiffusion to 0 if CRModel is 0?).

If a fix isn't easy, it may be worthwhile to include a note about this in the documentation.

Andrew

---

NSF Graduate Research Fellow

Columbia University

Department of Astronomy

[Brian O'Shea]

Hi Andrew,

That certainly doesn't sound right.  My suggestion would be to double-check it with Greg Bryan, but if you're correct (which I believe you are) it would be very easy to add a check at the end of ReadParameterFile.C that causes enzo to fail if it's set incorrectly.  For example:

  if(CRDiffusion>0 && CRModel==0){

    ENZO_FAIL("CRDiffusion can only be used if CRModel is turned on!\n");

  } 

I would note that there's also a parameter called CRFeedback that looks like it *can* be used with CRModel turned off, but *should not* be used in that circumstance.  Could you ask Greg about that as well?

Thanks,

Brian

--
You received this message because you are subscribed to the Google Groups "enzo-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email to enzo-dev+u...@googlegroups.com.
To post to this group, send email to enzo...@googlegroups.com.
Visit this group at https://groups.google.com/group/enzo-dev.
For more options, visit https://groups.google.com/d/optout.

[Andrew Emerick]

Hi Brian and Nathan,

I was planning on just adding something like:

if (CRModel == 0) CRDiffusion = 0;

into ReadParamaterFile.C, rather than a fail statement, since diffusion of something that doesn't exist doesn't make sense anyway (i.e. the CR parameters should do nothing if CRModel is off). But I figured this would be a band-aid fix. 

I think I found the issue, in EvolveLevel.C. It is currently calling "ComputeCRDiffusion" if "CRDiffusion" is ON (with no check to CRModel). In there, it computes the CR diffusion, but when CRModel is OFF, IdentifyPhysicalQuantities does not throw an error when looking for "CRNum", and returns CRNum = 0. Thus, If CRDiffusion is ON and CRModel off, it is currently computing the diffusion and applying it to the BaryonField[0], aka Density.

I'd be happy to submit a pull request if someone can let me know how to actually do that.

"I would note that there's also a parameter called CRFeedback that looks like it *can* be used with CRModel turned off, but *should not* be used in that circumstance.  Could you ask Greg about that as well?"

CRFeedback sets the fraction of energy dumped into cosmic rays during a supernova explosion, and is only used in star_feedback3 at the moment. It is contained within if statements checking if "CRModel" is on, 

Andrew

---

NSF Graduate Research Fellow

Columbia University

Department of Astronomy

[Brian O'Shea]

Hi Andrew,

Ah, interesting.  I suggest you add a fail statement rather than quietly correct it - while it's slightly annoying to the user, the enzo codebase tends to subscribe to the philosophy of "loudly inform the user they've made a mistake, and give them the chance to correct it" rather than "quietly set things to a set of parameters that makes sense."  The reason for this is that if a user actually wants the cosmic rays on, but forgot to turn CRModel = 1 when they turn on CRDiffusion, they could potentially waste a lot of computing time and human time before they realize their mistake.  So, it's better to loudly proclaim "this isn't right!" rather than assuming you know what the user's intention was.  Does that makes sense?

Regarding issuing a PR, I'd be happy to help out.  If you want to jump onto the yt IRC channel (see http://enzo-project.org/#help for instructions) or catch me on Google Chat (bwo...@gmail.com) I can walk you through the process.

--Brian

[Andrew Emerick]

Thanks Brian. I'll do both changes then, adding the EvolveLevel check and a fail statement in ReadParameterFile

Andrew

---

NSF Graduate Research Fellow

Columbia University

Department of Astronomy

[Greg Bryan]

Yes, I can confirm that this is a bug.  Andrew — would you mind fixing as Brian suggests and issuing a pull-request?   The CRFeedback is actually harmless with CRModel == 0 but it would probably be useful to check for that as well.

Thanks!

Greg