#2039: Meudon_Bin_NS update ------------------------------------+--------------------------------------- Reporter: knarf | Owner: Type: enhancement | Status: reopened Priority: minor | Milestone: Component: EinsteinToolkit thorn | Version: development version Resolution: | Keywords: parma ------------------------------------+---------------------------------------
Comment (by rhaas):
Ok. All I did was replacing CCTK_VInfo with CCTK_VError.
That one is fine. :-)
What about it? I don't think checking twice is a problem. LORENE should
contain a check, as it might be also used by other thorns than Meudon_Bin_NS. On the other hand, having a check in Meudon_Bin_NS allows for a better error message, since here you know which thorn actually caused the problem. This isn't critical for performance, and shouldn't fail in cases the others would not fail. Do you see a problem with checking twice?
There is a commit to Meudon_Bin_NS (64fc456 from 5 years ago) that goes along with the one in LORENE that does report the thorn (namely in the try{...}catch{...} block with the catch around line 245 {{{ } catch (ios::failure e) { CCTK_VWarn (CCTK_WARN_ABORT, __LINE__, __FILE__, CCTK_THORNSTRING, "Could not read initial data from file '%s': %s", filename, e.what()); } }}} So right now there are two parts of the code that do (almost) the same thing. I would want to keep only one of them to make maintenance easier. Otherwise depending on who looks at the code they may think that one or the other part of the code is actually the one that outputs errors when a file is not found (depending on whether they start reading the source code from the end or from the top of the file for example).
it would be good to add a comment to the commit message explaining
I intend to do this in the release notes. The reason being that I don't
think it would be visible enough in a commit message. I would now add something, but commit messages cannot be changed. Release message is certainly a place to put it, I would just put it also into the commit message so that we don't have to rely in persons remembering all the release messages.
how is now handling the case of a hot eos
The explicit '4' as eos index was replaced by a check for storage of
temperature and Y_e, and I call EOS_Omni_press accordingly. (I thought we discussed this change already. I'm sorry if I misremembered) There is a parameter to specify whether to take eps either from Lorene data, or to recalculate it, and depending on that key_temp is set to 0 or 1. Does that always work (and gives the very same results as before for the same correct set of parameter values)? I thought that before eg for a polytropic EOS it would pass havetemp=1 even though there was not storage for temperature?