sorainsm / library-of-lighting-models

Study of light modeling as a physics problem, with the purpose of implementing of a family of lighting models through a library.
0 stars 1 forks source link

Equations for IMs #13

Closed peter-michalski closed 4 years ago

peter-michalski commented 4 years ago

I am wondering if there should be equations for the IMs (derived from GD1). How does the programmer transform the inputs into the outputs listed in the IMs? I am not familiar with optics myself, if I had to program this I would not be sure how to proceed (even from an abstract viewpoint; that is, not considering the data structures or algorithms that will come during design).

I would like to get @smiths input on this as well in case I am wrong here - hoping to expand my understanding.

smiths commented 4 years ago

@peter-michalski, this is a great test of the documentation. If someone cannot implement the IMs, at least "abstractly", then the document isn't done. @sorainsm, I haven't finished my review of your document, but I can see what @peter-michalski is getting at. Your Output field lists the output variables, but it doesn't say anything about how they are calculated. It looks like information is missing.

sharyuwu commented 4 years ago

@sorainsm I also agree with the point doctor smith and peter points. It looks confusing without the equation/relationship between input and output in your IM. Moreover, it a bit hard to tell how does the data definition drive the Instance Model. I will post an issue about this question.

sorainsm commented 4 years ago

Equations added to IMs in cb95e52e54dcfd0b1df559731131bd8280acd53e. Also revised their structure to make the input and output types explicit.

@peter-michalski can you take a look at this new structure and let me know if it's more clear how the IMs would be implemented?

@smiths can you take a look at the structures and let me know if it's too close to code?

sorainsm commented 4 years ago

Closing issue as the IMs have been resolved and I'm unlikely to get feedback at this point.