Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

legacy units and rounding constants #432

Open
cattabiani opened this issue Nov 19, 2020 · 3 comments
Open

legacy units and rounding constants #432

cattabiani opened this issue Nov 19, 2020 · 3 comments
Assignees
Labels
codegen Code generation backend

Comments

@cattabiani
Copy link
Contributor

I report a cvf issue here as it is useful to decide how to tackle this problem:

In hippocampus/mod/cagk.mod exposes a new, related, possible missmatch between nmodl and mod2c (or neuron) behaviors.

In that file we find:

UNITS {
	FARADAY = (faraday)  (kilocoulombs)
	R = 8.313424 (joule/degC)
}

Faraday is taken from the internal library while R is set as custom value. If we use legacy units, in mod2c (or neuron) we get in the .c file:

#define FARADAY _nrnunit_FARADAY[_nrnunit_use_legacy_]
static double _nrnunit_FARADAY[2] = {0x1.81f0fae775426p+6, 96.4853}; /* 96.4853321233100303 */
 static double R = 8.313424;

Let's notice that Faraday (96.4853) has 6 digits while R (8.313424) has 7 digits. This is due to the fact that faraday is hard-coded as exactly that number (96.4853) in legacy units and R is kept as a string and copy-pasted in the .c file.

Meanwhile, in nmodl, doubles are stored as doubles and printed using this line (from codegen_c_visitor):

std::string format_string = "static const double {} = {:g};";

In the fmt documentation, for g we get:

'g'	
General format. For a given precision p >= 1, this rounds the number to p significant digits and then formats the result in either fixed-point format or in scientific notation, depending on its magnitude.

As last, note that the standard legacy faraday constant is taken from this value:

static const double FARADAY = 96.4853321233100184

Now, let's check what happens for FARADAY in nmodl:

  • the full value is loaded from the library (96.4853321233100184)
  • static const double {} = {:g} chops it to 6 digits (default value)
  • we get the correct legacy value of 96.4853

For the custom R the process is similar:

  • the value is loaded from the mod file (8.313424)
  • {quote}static const double {} = {:g}{quote} chops it to 6 digits (default value)
  • we get the incorrect legacy value of 8.31342

Unfortunately, this means that we cannot simply tamper with fmt precision like:

std::string format_string = "static const double {} = {:.18g};";

because faraday and R require different precisions: either we chop too much R or we chop too little faraday.

Storing R as a string would help but we need to find a way to chop faraday if legacy units is on

@cattabiani
Copy link
Contributor Author

cattabiani commented Apr 23, 2021

It looks like cagk still has some problems.

neuron:

static double _nrnunit_FARADAY[2] = {0x1.81f0fae775425p+6, 96.4853}; /* 96.4853321233100161 */
 static double R = 8.313424;

coreneuron-nmodl:

    static const double FARADAY = 96.4853;
    static const double R = 8.31342;

the R values is not correct in coreneuron-nmodl

@pramodk
Copy link
Contributor

pramodk commented Apr 26, 2021

cc: @alkino (see above)

@alkino
Copy link
Member

alkino commented Jul 19, 2021

discussed here: neuronsimulator/nrn#1226

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
codegen Code generation backend
Projects
None yet
Development

No branches or pull requests

4 participants