Change mass density to number density

[pcubillos]

Nowhere in the code the mass density (g cm-3) is used 'as is'. A more natural unit is the number density. This will simplify the code, and make it more standard with the units used in the literature (e.g., the units in the opacity file will be cm2/mol instead of cm2/g).

[joeharr4]

Where in the code is the density used? Is it always converted to number density wherever it is used? How big a change is this, and does it affect just config files or C code? While in principle this change looks like a good idea, it has "bug" written all over it, if you forget even one instance that needs changing. This is the kind of thing where tests help!

--jh--

X-Spam-Status: No, score=-4.8 required=4.0 tests=BAYES_00,HTML_IMAGE_ONLY_12, HTML_MESSAGE,RCVD_IN_DNSWL_HI,RCVD_IN_MSPIKE_H3,RCVD_IN_MSPIKE_WL, T_DKIM_INVALID autolearn=ham autolearn_force=no version=3.4.0 Date: Mon, 20 Jul 2015 08:08:22 -0700 From: Patricio Cubillos notifications@github.com Reply-To: exosports/transit reply@reply.github.com To: exosports/transit transit@noreply.github.com Subject: [transit] Change mass density to number density (#88) Content-Type: multipart/alternative; boundary="--==_mimepart_55ad0ee6aea35_14f93fa5532df2c098319"; charset=UTF-8

[1:text/plain Hide]

Nowhere in the code the mass density (g cm-3) is used 'as is'. A more natural unit is the number density. This will simplify the code, and make it more standard with the units used in the literature (e.g., the units in the opacity file will be cm2/mol instead of cm2/g).

---

Reply to this email directly or view it on GitHub: https://github.com/exosports/transit/issues/88

[2:text/html Show]

[pcubillos]

Where in the code is the density used? Many places.

Is it always converted to number density wherever it is used? Yes, always.

How big a change is this, and does it affect just config files or C code? The other way around, it affect C code but not any config files. I would say it deals with some 4-8 source files, a few structure definitions, a many places in the documentation.

While in principle this change looks like a good idea, it has "bug" written all over it, if you forget even one instance that needs changing. This is the kind of thing where tests help! I know, but I also think this is an important change. We want others to use this code. Making a new paradigm to handle a problem that others have already solved must be justified (which is not the case here). I will make things easier and more welcoming for others when they want to use Transit, it will make things easier to us when we want to compare against other results from the literature. This is a risk I'm willing to take.

[joeharr4]

I know, but I also think this is an important change. We want others to use this code. Making a new paradigm to handle a problem that others have already solved must be justified (which is not the case here). I will make things easier and more welcoming for others when they want to use Transit, it will make things easier to us when we want to compare against other results from the literature.

This is a risk I'm willing to take.

I am wondering why Pato Rojo did it that way in the first place. Did you ask? But, I agree it's a good idea to do this. The question is when, who, and how to ensure it's done right.

--jh--