search

ISET / iset3d-v3

LEGACY: Replaced by iset3d (v4) -- Read a PBRT file. Edit the parameters. Render an ISET scene or optical image.
MIT License
36 stars 8 forks source link

recipeSet issues on materials branch #33

Closed DavidBrainard closed 3 years ago

DavidBrainard commented 3 years ago

This may be known and understood. The function recipeSet has many options. For some, it returns the empty matrix. This is because the first thing it does is set its return variable, out, equal to []. Then, for many of its options, it operates on its input recipe (thisR), but never sets the result to out.

It may be this is because receipeSet is on its way out the door as part of the materials branch. I encountered it because I was going to update t_piIntro_simpletransforms to work on the materials branch (seemed useful). But this call early on breaks:

thisR = recipeSet(thisR,'outputfile',... fullfile(piRootPath,'local','coloredCube','coloredCube.pbrt'));

Since it may be there is already a plan on how to handle this, I decided to leave it alone.

digital-pro commented 3 years ago

David – Great timing. I just tripped over the same thing this morning and was wondering what was up with the two very different return strategies. I was going to ask Brian later today. Drove me nuts for a little while until I figured out the issue.

In the larger scheme of things now that Matlab supports code to implement properties, in future I’m not sure we need set/get methods. I guess maybe those were left over from before Recipes were objects?

From: David Brainard notifications@github.com Sent: Monday, January 04, 2021 12:18 PM To: ISET/iset3d iset3d@noreply.github.com Cc: Subscribed subscribed@noreply.github.com Subject: [ISET/iset3d] recipeSet issues on materials branch (#33)

This may be known and understood. The function recipeSet has many options. For some, it returns the empty matrix. This is because the first thing it does is set its return variable, out, equal to []. Then, for many of its options, it operates on its input recipe (thisR), but never sets the result to out.

It may be this is because receipeSet is on its way out the door as part of the materials branch. I encountered it because I was going to update t_piIntro_simpletransforms to work on the materials branch (seemed useful). But this call early on breaks:

thisR = recipeSet(thisR,'outputfile',... fullfile(piRootPath,'local','coloredCube','coloredCube.pbrt'));

Since it may be there is already a plan on how to handle this, I decided to leave it alone.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/ISET/iset3d/issues/33 , or unsubscribe https://github.com/notifications/unsubscribe-auth/ABJ4N3C6CBTZSZN6ZJXEYHLSYIO6XANCNFSM4VTQCSCA . https://github.com/notifications/beacon/ABJ4N3BEI5KWY3PW7VKRAR3SYIO6XA5CNFSM4VTQCSCKYY3PNVWWK3TUL52HS4DFUVEXG43VMWVGG33NNVSW45C7NFSM4LTEP4DQ.gif

DavidBrainard commented 3 years ago

This was enough of a clue to get me to try thisR.set('outputfile', fullfile(piRootPath,'local','coloredCube','coloredCube.pbrt')); which appears to work.

Probably recipeSet is being deprecated and a fix like this is appropriate in places where it appears.

From: David Cardinal notifications@github.com Reply-To: ISET/iset3d reply@reply.github.com Date: Monday, January 4, 2021 at 3:31 PM To: ISET/iset3d iset3d@noreply.github.com Cc: "Brainard, David H" brainard@psych.upenn.edu, Author author@noreply.github.com Subject: Re: [ISET/iset3d] recipeSet issues on materials branch (#33)

David – Great timing. I just tripped over the same thing this morning and was wondering what was up with the two very different return strategies. I was going to ask Brian later today. Drove me nuts for a little while until I figured out the issue.

In the larger scheme of things now that Matlab supports code to implement properties, in future I’m not sure we need set/get methods. I guess maybe those were left over from before Recipes were objects?

From: David Brainard notifications@github.com Sent: Monday, January 04, 2021 12:18 PM To: ISET/iset3d iset3d@noreply.github.com Cc: Subscribed subscribed@noreply.github.com Subject: [ISET/iset3d] recipeSet issues on materials branch (#33)

This may be known and understood. The function recipeSet has many options. For some, it returns the empty matrix. This is because the first thing it does is set its return variable, out, equal to []. Then, for many of its options, it operates on its input recipe (thisR), but never sets the result to out.

It may be this is because receipeSet is on its way out the door as part of the materials branch. I encountered it because I was going to update t_piIntro_simpletransforms to work on the materials branch (seemed useful). But this call early on breaks:

thisR = recipeSet(thisR,'outputfile',...

Since it may be there is already a plan on how to handle this, I decided to leave it alone.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/ISET/iset3d/issues/33 , or unsubscribe https://github.com/notifications/unsubscribe-auth/ABJ4N3C6CBTZSZN6ZJXEYHLSYIO6XANCNFSM4VTQCSCA . https://github.com/notifications/beacon/ABJ4N3BEI5KWY3PW7VKRAR3SYIO6XA5CNFSM4VTQCSCKYY3PNVWWK3TUL52HS4DFUVEXG43VMWVGG33NNVSW45C7NFSM4LTEP4DQ.gif

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHubhttps://github.com/ISET/iset3d/issues/33#issuecomment-754201473, or unsubscribehttps://github.com/notifications/unsubscribe-auth/AAI24ZKXPMMVC7GYYU3KSYTSYIQSLANCNFSM4VTQCSCA.

DavidBrainard commented 3 years ago

I got further in my efforts to revive t_piIntro_simpletransforms, but code that appears to work in tls_assets.mlx does not seem to do so in the simpletransforms script. For me this crashes out when I try to set the rotation on the cube, line 48 of the script.

It may be that the coloredCube recipe is not set up right, although it reads in and I can print the tree structure out just fine.

Maybe I'm not generalizing from the tls_assets script correctly, or maybe this is revealing some buglet that wasn't tripped when a similar thing was done with the simple scene recipe.

Best,

David

From: David Cardinal notifications@github.com Reply-To: ISET/iset3d reply@reply.github.com Date: Monday, January 4, 2021 at 3:31 PM To: ISET/iset3d iset3d@noreply.github.com Cc: "Brainard, David H" brainard@psych.upenn.edu, Author author@noreply.github.com Subject: Re: [ISET/iset3d] recipeSet issues on materials branch (#33)

David – Great timing. I just tripped over the same thing this morning and was wondering what was up with the two very different return strategies. I was going to ask Brian later today. Drove me nuts for a little while until I figured out the issue.

In the larger scheme of things now that Matlab supports code to implement properties, in future I’m not sure we need set/get methods. I guess maybe those were left over from before Recipes were objects?

From: David Brainard notifications@github.com Sent: Monday, January 04, 2021 12:18 PM To: ISET/iset3d iset3d@noreply.github.com Cc: Subscribed subscribed@noreply.github.com Subject: [ISET/iset3d] recipeSet issues on materials branch (#33)

This may be known and understood. The function recipeSet has many options. For some, it returns the empty matrix. This is because the first thing it does is set its return variable, out, equal to []. Then, for many of its options, it operates on its input recipe (thisR), but never sets the result to out.

It may be this is because receipeSet is on its way out the door as part of the materials branch. I encountered it because I was going to update t_piIntro_simpletransforms to work on the materials branch (seemed useful). But this call early on breaks:

thisR = recipeSet(thisR,'outputfile',... fullfile(piRootPath,'local','coloredCube','coloredCube.pbrt'));

Since it may be there is already a plan on how to handle this, I decided to leave it alone.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/ISET/iset3d/issues/33 , or unsubscribe https://github.com/notifications/unsubscribe-auth/ABJ4N3C6CBTZSZN6ZJXEYHLSYIO6XANCNFSM4VTQCSCA . https://github.com/notifications/beacon/ABJ4N3BEI5KWY3PW7VKRAR3SYIO6XA5CNFSM4VTQCSCKYY3PNVWWK3TUL52HS4DFUVEXG43VMWVGG33NNVSW45C7NFSM4LTEP4DQ.gif

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHubhttps://github.com/ISET/iset3d/issues/33#issuecomment-754201473, or unsubscribehttps://github.com/notifications/unsubscribe-auth/AAI24ZKXPMMVC7GYYU3KSYTSYIQSLANCNFSM4VTQCSCA.

digital-pro commented 3 years ago

David – Try this:

thisR.set('asset', thisAsset.name, 'rotate', [0, 0, 45]);

DavidBrainard commented 3 years ago

Great, works!

What led me astray was that I overgeneralized from the translate example in tls_assets (pasted below), and assumed that rotate could take an asset itself.

In general, I realize I'm not clear on what may be passed for the asset, when. And indeed, not clear on when we apply transformations to an asset itself and when to the parent (again, as distinguished in the translate and rotate examples in tls_assets.)

Best,

David

% This is the object representing the yellow man assetName = 'figure_6m_O';

% In this example, we find the branch node that is just above the yellow % man, representing its position, rotation and such. thisAsset = thisR.get('asset parent id',assetName);

% We add a translation, moving yellow man 2 meters in the z direction. translateBranch = thisR.set('asset', thisAsset, 'translate', [0, 0, -2]);

From: David Cardinal notifications@github.com Reply-To: ISET/iset3d reply@reply.github.com Date: Monday, January 4, 2021 at 4:57 PM To: ISET/iset3d iset3d@noreply.github.com Cc: "Brainard, David H" brainard@psych.upenn.edu, Author author@noreply.github.com Subject: Re: [ISET/iset3d] recipeSet issues on materials branch (#33)

David – Try this:

thisR.set('asset', thisAsset.name, 'rotate', [0, 0, 45]);

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHubhttps://github.com/ISET/iset3d/issues/33#issuecomment-754245363, or unsubscribehttps://github.com/notifications/unsubscribe-auth/AAI24ZL5ECTMU2AU7P5LUJ3SYI2WDANCNFSM4VTQCSCA.

digital-pro commented 3 years ago

One thing that might be helpful in the scripts/tutorials is if we try to distinguish between things and their names/IDs (I’m probably as guilty as the next person, but maybe I can make it a New Year’s Resolution). For example the code David was looking at below includes:

thisAsset = thisR.get('asset parent id',assetName);

% We add a translation, moving yellow man 2 meters in the z direction. translateBranch = thisR.set('asset', thisAsset, 'translate', [0, 0, -2]);

Which looking at it I’d interpret as getting an object handle/instance back from the get call and passing it. Except it is actually just an ID for the asset in the tree. Does this make sense?

From: David Brainard notifications@github.com Sent: Monday, January 04, 2021 2:06 PM To: ISET/iset3d iset3d@noreply.github.com Cc: David Cardinal github@cardinalphoto.com; Comment comment@noreply.github.com Subject: Re: [ISET/iset3d] recipeSet issues on materials branch (#33)

Great, works!

What led me astray was that I overgeneralized from the translate example in tls_assets (pasted below), and assumed that rotate could take an asset itself.

In general, I realize I'm not clear on what may be passed for the asset, when. And indeed, not clear on when we apply transformations to an asset itself and when to the parent (again, as distinguished in the translate and rotate examples in tls_assets.)

Best,

David

% This is the object representing the yellow man assetName = 'figure_6m_O';

% In this example, we find the branch node that is just above the yellow % man, representing its position, rotation and such. thisAsset = thisR.get('asset parent id',assetName);

% We add a translation, moving yellow man 2 meters in the z direction. translateBranch = thisR.set('asset', thisAsset, 'translate', [0, 0, -2]);

From: David Cardinal <notifications@github.com mailto:notifications@github.com > Reply-To: ISET/iset3d <reply@reply.github.com mailto:reply@reply.github.com > Date: Monday, January 4, 2021 at 4:57 PM To: ISET/iset3d <iset3d@noreply.github.com mailto:iset3d@noreply.github.com > Cc: "Brainard, David H" <brainard@psych.upenn.edu mailto:brainard@psych.upenn.edu >, Author <author@noreply.github.com mailto:author@noreply.github.com > Subject: Re: [ISET/iset3d] recipeSet issues on materials branch (#33)

David – Try this:

thisR.set('asset', thisAsset.name, 'rotate', [0, 0, 45]);

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHubhttps://github.com/ISET/iset3d/issues/33#issuecomment-754245363, or unsubscribehttps://github.com/notifications/unsubscribe-auth/AAI24ZL5ECTMU2AU7P5LUJ3SYI2WDANCNFSM4VTQCSCA.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/ISET/iset3d/issues/33#issuecomment-754251721 , or unsubscribe https://github.com/notifications/unsubscribe-auth/ABJ4N3BNYT6H6LEZZ2ON5LTSYI3S7ANCNFSM4VTQCSCA . https://github.com/notifications/beacon/ABJ4N3B7ZUYSGMJCJDDX5LDSYI3S7A5CNFSM4VTQCSCKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOFT2PPSI.gif

DavidBrainard commented 3 years ago

I've now reconstructed more of the path that led me astray.

I had started with the name, but I had also started trying to rotation what I thought was the whole cube, name '003ID_Cube_B'. Using this as the asset name in simpletransforms still crashes, but I guess for a reason other than that it is the name and not the asset. That led me to try passing the asset rather than the name ...

Using '005ID_Cube_O' runs but only rotates one side of the cube (because, well, it is one side).

So, more mysteries. Can one only rotate a leaf?

From: David Cardinal notifications@github.com Reply-To: ISET/iset3d reply@reply.github.com Date: Monday, January 4, 2021 at 5:42 PM To: ISET/iset3d iset3d@noreply.github.com Cc: "Brainard, David H" brainard@psych.upenn.edu, Author author@noreply.github.com Subject: Re: [ISET/iset3d] recipeSet issues on materials branch (#33)

One thing that might be helpful in the scripts/tutorials is if we try to distinguish between things and their names/IDs (I’m probably as guilty as the next person, but maybe I can make it a New Year’s Resolution). For example the code David was looking at below includes:

thisAsset = thisR.get('asset parent id',assetName);

% We add a translation, moving yellow man 2 meters in the z direction. translateBranch = thisR.set('asset', thisAsset, 'translate', [0, 0, -2]);

Which looking at it I’d interpret as getting an object handle/instance back from the get call and passing it. Except it is actually just an ID for the asset in the tree. Does this make sense?

From: David Brainard notifications@github.com Sent: Monday, January 04, 2021 2:06 PM To: ISET/iset3d iset3d@noreply.github.com Cc: David Cardinal github@cardinalphoto.com; Comment comment@noreply.github.com Subject: Re: [ISET/iset3d] recipeSet issues on materials branch (#33)

Great, works!

What led me astray was that I overgeneralized from the translate example in tls_assets (pasted below), and assumed that rotate could take an asset itself.

In general, I realize I'm not clear on what may be passed for the asset, when. And indeed, not clear on when we apply transformations to an asset itself and when to the parent (again, as distinguished in the translate and rotate examples in tls_assets.)

Best,

David

% This is the object representing the yellow man assetName = 'figure_6m_O';

% In this example, we find the branch node that is just above the yellow % man, representing its position, rotation and such. thisAsset = thisR.get('asset parent id',assetName);

% We add a translation, moving yellow man 2 meters in the z direction. translateBranch = thisR.set('asset', thisAsset, 'translate', [0, 0, -2]);

From: David Cardinal <notifications@github.com mailto:notifications@github.com > Reply-To: ISET/iset3d <reply@reply.github.com mailto:reply@reply.github.com > Date: Monday, January 4, 2021 at 4:57 PM To: ISET/iset3d <iset3d@noreply.github.com mailto:iset3d@noreply.github.com > Cc: "Brainard, David H" <brainard@psych.upenn.edu mailto:brainard@psych.upenn.edu >, Author <author@noreply.github.com mailto:author@noreply.github.com > Subject: Re: [ISET/iset3d] recipeSet issues on materials branch (#33)

David – Try this:

thisR.set('asset', thisAsset.name, 'rotate', [0, 0, 45]);

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHubhttps://github.com/ISET/iset3d/issues/33#issuecomment-754245363, or unsubscribehttps://github.com/notifications/unsubscribe-auth/AAI24ZL5ECTMU2AU7P5LUJ3SYI2WDANCNFSM4VTQCSCA.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/ISET/iset3d/issues/33#issuecomment-754251721 , or unsubscribe https://github.com/notifications/unsubscribe-auth/ABJ4N3BNYT6H6LEZZ2ON5LTSYI3S7ANCNFSM4VTQCSCA . https://github.com/notifications/beacon/ABJ4N3B7ZUYSGMJCJDDX5LDSYI3S7A5CNFSM4VTQCSCKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOFT2PPSI.gif

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHubhttps://github.com/ISET/iset3d/issues/33#issuecomment-754267146, or unsubscribehttps://github.com/notifications/unsubscribe-auth/AAI24ZMS66O7PAETAHHWMX3SYI75ZANCNFSM4VTQCSCA.

DavidBrainard commented 3 years ago

Found and fixed the underlying bug in piAssetRotate. There was a vestigial call of form thisR = thisR.set(...) in the part of the code where a branch was being rotated. This didn't show up in tests where a leaf was being rotated (as in tls_assets).

Also fixed the examples in piAssetRotate and piAssetTranslate so they run, and did a little comment cleaning.

Commit 9ef02ffa on the materials branch.

Closing this issue.

wandell commented 3 years ago

Good. DJC and I are just dealing with this issue at a slightly more global scale. Will update when we think our solution is working correctly.

Brian

From: David Brainard notifications@github.com Date: Tuesday, January 5, 2021 at 3:24 PM To: ISET/iset3d iset3d@noreply.github.com Cc: Subscribed subscribed@noreply.github.com Subject: Re: [ISET/iset3d] recipeSet issues on materials branch (#33)

Found and fixed the underlying bug in piAssetRotate. There was a vestigial call of form thisR = thisR.set(...) in the part of the code where a branch was being rotated. This didn't show up in tests where a leaf was being rotated (as in tls_assets).

Also fixed the examples in piAssetRotate and piAssetTranslate so they run, and did a little comment cleaning.

Commit 9ef02ffa on the materials branch.

Closing this issue.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHubhttps://github.com/ISET/iset3d/issues/33#issuecomment-754961747, or unsubscribehttps://github.com/notifications/unsubscribe-auth/AAOAQWIGRLVMCJBKOXDKSKDSYONSJANCNFSM4VTQCSCA.

digital-pro commented 3 years ago

David – That’s great. While you were doing that, Brian and I were taking a look at recipeSet in general. There has been some understandable confusion (including yours and mine) over when it returns a recipe (which it probably doesn’t need to, since it is a call-by-reference class) and when it returns some type of other value. So we modified it to return a tuple of [thisR, out]. So callers expecting a recipe will get one. Callers expecting some other type of return need to be modified to [~, out]. Seems like the best short-term fix. We ran through the asset test script and fixed the one caller that broke. Holler if you find anything awful that we broke! – David

From: David Brainard notifications@github.com Sent: Tuesday, January 05, 2021 3:24 PM To: ISET/iset3d iset3d@noreply.github.com Cc: David Cardinal github@cardinalphoto.com; Comment comment@noreply.github.com Subject: Re: [ISET/iset3d] recipeSet issues on materials branch (#33)

Found and fixed the underlying bug in piAssetRotate. There was a vestigial call of form thisR = thisR.set(...) in the part of the code where a branch was being rotated. This didn't show up in tests where a leaf was being rotated (as in tls_assets).

Also fixed the examples in piAssetRotate and piAssetTranslate so they run, and did a little comment cleaning.

Commit 9ef02ffa on the materials branch.

Closing this issue.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/ISET/iset3d/issues/33#issuecomment-754961747 , or unsubscribe https://github.com/notifications/unsubscribe-auth/ABJ4N3FH4WNZ3M4LALJ2KVLSYONSJANCNFSM4VTQCSCA . https://github.com/notifications/beacon/ABJ4N3D5GIKGAV47XCETWWTSYONSJA5CNFSM4VTQCSCKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOFT742UY.gif

DavidBrainard commented 3 years ago

I went in and fixed all the tutorials in assets for this change. There was also one similar fix in recipeSet itself.

David

From: David Cardinal notifications@github.com Reply-To: ISET/iset3d reply@reply.github.com Date: Tuesday, January 5, 2021 at 7:16 PM To: ISET/iset3d iset3d@noreply.github.com Cc: "Brainard, David H" brainard@psych.upenn.edu, State change state_change@noreply.github.com Subject: Re: [ISET/iset3d] recipeSet issues on materials branch (#33)

David – That’s great. While you were doing that, Brian and I were taking a look at recipeSet in general. There has been some understandable confusion (including yours and mine) over when it returns a recipe (which it probably doesn’t need to, since it is a call-by-reference class) and when it returns some type of other value. So we modified it to return a tuple of [thisR, out]. So callers expecting a recipe will get one. Callers expecting some other type of return need to be modified to [~, out]. Seems like the best short-term fix. We ran through the asset test script and fixed the one caller that broke. Holler if you find anything awful that we broke! – David

From: David Brainard notifications@github.com Sent: Tuesday, January 05, 2021 3:24 PM To: ISET/iset3d iset3d@noreply.github.com Cc: David Cardinal github@cardinalphoto.com; Comment comment@noreply.github.com Subject: Re: [ISET/iset3d] recipeSet issues on materials branch (#33)

Found and fixed the underlying bug in piAssetRotate. There was a vestigial call of form thisR = thisR.set(...) in the part of the code where a branch was being rotated. This didn't show up in tests where a leaf was being rotated (as in tls_assets).

Also fixed the examples in piAssetRotate and piAssetTranslate so they run, and did a little comment cleaning.

Commit 9ef02ffa on the materials branch.

Closing this issue.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/ISET/iset3d/issues/33#issuecomment-754961747 , or unsubscribe https://github.com/notifications/unsubscribe-auth/ABJ4N3FH4WNZ3M4LALJ2KVLSYONSJANCNFSM4VTQCSCA . https://github.com/notifications/beacon/ABJ4N3D5GIKGAV47XCETWWTSYONSJA5CNFSM4VTQCSCKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOFT742UY.gif

— You are receiving this because you modified the open/close state. Reply to this email directly, view it on GitHubhttps://github.com/ISET/iset3d/issues/33#issuecomment-754979478, or unsubscribehttps://github.com/notifications/unsubscribe-auth/AAI24ZMNV7NT2PWN46XLXCTSYOTVRANCNFSM4VTQCSCA.

digital-pro commented 3 years ago

Thanks! – David C

From: David Brainard notifications@github.com Sent: Wednesday, January 06, 2021 8:37 AM To: ISET/iset3d iset3d@noreply.github.com Cc: David Cardinal github@cardinalphoto.com; Comment comment@noreply.github.com Subject: Re: [ISET/iset3d] recipeSet issues on materials branch (#33)

I went in and fixed all the tutorials in assets for this change. There was also one similar fix in recipeSet itself.

David

From: David Cardinal <notifications@github.com mailto:notifications@github.com > Reply-To: ISET/iset3d <reply@reply.github.com mailto:reply@reply.github.com > Date: Tuesday, January 5, 2021 at 7:16 PM To: ISET/iset3d <iset3d@noreply.github.com mailto:iset3d@noreply.github.com > Cc: "Brainard, David H" <brainard@psych.upenn.edu mailto:brainard@psych.upenn.edu >, State change <state_change@noreply.github.com mailto:state_change@noreply.github.com > Subject: Re: [ISET/iset3d] recipeSet issues on materials branch (#33)

David – That’s great. While you were doing that, Brian and I were taking a look at recipeSet in general. There has been some understandable confusion (including yours and mine) over when it returns a recipe (which it probably doesn’t need to, since it is a call-by-reference class) and when it returns some type of other value. So we modified it to return a tuple of [thisR, out]. So callers expecting a recipe will get one. Callers expecting some other type of return need to be modified to [~, out]. Seems like the best short-term fix. We ran through the asset test script and fixed the one caller that broke. Holler if you find anything awful that we broke! – David

From: David Brainard <notifications@github.com mailto:notifications@github.com > Sent: Tuesday, January 05, 2021 3:24 PM To: ISET/iset3d <iset3d@noreply.github.com mailto:iset3d@noreply.github.com > Cc: David Cardinal <github@cardinalphoto.com mailto:github@cardinalphoto.com >; Comment <comment@noreply.github.com mailto:comment@noreply.github.com > Subject: Re: [ISET/iset3d] recipeSet issues on materials branch (#33)

Found and fixed the underlying bug in piAssetRotate. There was a vestigial call of form thisR = thisR.set(...) in the part of the code where a branch was being rotated. This didn't show up in tests where a leaf was being rotated (as in tls_assets).

Also fixed the examples in piAssetRotate and piAssetTranslate so they run, and did a little comment cleaning.

Commit 9ef02ffa on the materials branch.

Closing this issue.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/ISET/iset3d/issues/33#issuecomment-754961747 , or unsubscribe https://github.com/notifications/unsubscribe-auth/ABJ4N3FH4WNZ3M4LALJ2KVLSYONSJANCNFSM4VTQCSCA . https://github.com/notifications/beacon/ABJ4N3D5GIKGAV47XCETWWTSYONSJA5CNFSM4VTQCSCKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOFT742UY.gif

— You are receiving this because you modified the open/close state. Reply to this email directly, view it on GitHubhttps://github.com/ISET/iset3d/issues/33#issuecomment-754979478, or unsubscribehttps://github.com/notifications/unsubscribe-auth/AAI24ZMNV7NT2PWN46XLXCTSYOTVRANCNFSM4VTQCSCA.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/ISET/iset3d/issues/33#issuecomment-755412892 , or unsubscribe https://github.com/notifications/unsubscribe-auth/ABJ4N3DGEVFCKSJ223YM54TSYSGSTANCNFSM4VTQCSCA . https://github.com/notifications/beacon/ABJ4N3GT46HPRJPEG7YNAJTSYSGSTA5CNFSM4VTQCSCKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOFUDK7HA.gif