search

ladybug-tools / honeybee-radiance-command

🐝 ⚡️ :abc: Honeybee wrapper around Radiance commands which is used by honeybee-radiance
https://www.ladybug.tools/honeybee-radiance-command/docs/
GNU Affero General Public License v3.0
1 stars 7 forks source link

Rcontrib options sequence is incorrect #197

Closed sariths closed 3 years ago

sariths commented 3 years ago

For example, the following two commands do not produce the same result:

rcontrib -I -o %s.mtx -M matList.txt -ad 10000 -lw 0.0001 scene.oct < pointsSingle.txt

rcontrib -I -M matList.txt -ad 10000 -lw 0.0001 -o %s.mtx scene.oct < pointsSingle.txt

However, the way the options are setup right now, there is no way to ensure that the output format -o %s.mtx always comes before the material definitions -M matList.txt

sariths commented 3 years ago

Just to add to my previous comment, from the docs... "The most recent −p, −b, −bn and −o options to the left of each −m setting are the ones used for that modifier. The ordering of other options is unimportant, except for −x and −y if the −c is zero, when they control the resolution string produced in the corresponding output." I don't think we need to burn down the entire house to fix this. I can override the toRadiance method from the OptionCollection parent-clss inside Rcontrib options so that the change/modification is local and does not break anything else. cc: @chriswmackey , @devngc , @mostaphaRoudsari

mostaphaRoudsari commented 3 years ago

Thanks, @sariths! I missed this one. Overriding the method should address this issue.

chriswmackey commented 3 years ago

I'm all in favor of overriding the inherited method!

sariths commented 3 years ago

Was addressed by https://github.com/ladybug-tools/honeybee-radiance-command/pull/200