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

Define preferred notation of units in models #356

Closed
PTraeder opened this issue May 19, 2017 · 8 comments
Closed

Define preferred notation of units in models #356

PTraeder opened this issue May 19, 2017 · 8 comments

Comments

@PTraeder
Copy link
Contributor

PTraeder commented May 19, 2017

Since #348 it is possible to write 1 ms as shorthand for 1*ms.
Additionally, predefined variables for each unit have been registered, so that notations like foo/ms or 1 ms * mV have become possible.
In the case foo/ms the factor 1 is implied for the ms unit.

@jougs and @heplesser expressed that they would like to see literals in front of every unit.
I have some questions regarding this:

A verbose interpretation of this demand could look like:
1 ms / (1 mV * 1 nS) vs. ms/(mv*nS)
foo * 1 nS vs. foo * nS
foo / 1 nS vs. foo / nS
1 nS * foo vs. nS * foo
1 nS / foo vs. nS / foo

I have concerns regarding the legibility of the first case in large expressions.
An alternative could be that we only require one literal per "compound unit", where a compound unit would be a factor in an expression that consists only of units, e.g.:
1 ms/(mV*nS)

Which version, if any, would you prefer?

Are there any other conventions wrt. notation of units that you would like to see?

Also would you like to see some form warning if the conventions we agree upon are breached?

@PTraeder
Copy link
Contributor Author

@Silmathoron
Copy link
Member

I'm not sure @heplesser said he wanted to see litterals in front of every unit...
I think his point was that, in the examples (for the sake of clarity), the unit should always be attached to the variable it belongs to.

E.g. for PSConInit_E nS/ms = 1.0 nS * e / tau_syn_ex, their logic would be to keep it that way, since it is composed by:

  • a normalisation factor 1 nS
  • a dimensionless value e
  • a time which comes with its own unit value.

@PTraeder
Copy link
Contributor Author

I honestly do not understand the pattern you are describing.

@jougs
Copy link
Contributor

jougs commented May 20, 2017

What @heplesser and I suggested (and what @Silmathoron propably meant) is that we should always have one single normalization factor (usually 1 as an integer, not real) in compound initialization expressions, instead of none at all.

In other words, we rather want 1 ns * e / tau_syn_ex instead of just ns * e / tau_syn_ex as it currently is used in the changes introduced in #348 . In the same spirit, we also want compound initializing expressions to be 1 ms / (mv * nS) instead of 1 ms / (1 mV * 1 nS) or ms / (mV * nS). This is in accordance with common practice in physics and neuroscience.

I think this is all possible with the implementation proposed in #348. The code does just not reflect this convention or at least this is just not used consistently (cf. old and new line 89 of models/aeif_cond_alpha.nestmlin the linked PR).

@PTraeder: does this clear up the confusion?

@PTraeder
Copy link
Contributor Author

Yes. Thank you, I can work with that.

@heplesser
Copy link

I just want to confirm that @jougs understood me correctly.

@Silmathoron
Copy link
Member

Is there anything left to do with this issue or should we close it?

@jougs
Copy link
Contributor

jougs commented Jul 30, 2019

@Silmathoron: I don't think so. Thanks for the heads up.

@jougs jougs closed this as completed Jul 30, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants