Skip to content
This repository has been archived by the owner on Sep 20, 2024. It is now read-only.

Bugfix: Houdini render split bugs #6037

Merged
merged 12 commits into from
Dec 21, 2023

Conversation

MustafaJafar
Copy link
Contributor

@MustafaJafar MustafaJafar commented Dec 8, 2023

Changelog Description

This PR is a follow up PR to #5420

This PR does:

  1. refactor get_output_parameter to what is used to be.
  2. fix a bug with split render
  3. rename exportJob flag to split_render

Testing notes:

  1. open houdini
  2. submit different product types, they should work

@ynbot ynbot added host: Houdini type: bug Something isn't working size/S Denotes a PR changes 100-499 lines, ignoring general files labels Dec 8, 2023
Copy link
Member

@moonyuet moonyuet left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tried testing with OP first before testing in AYON(for easier to spot the issue), my vray isn't compatible with my current version of houdini and i dont have Arnold llicense so can't really test on these two.
r without exportJob in the instance.data.
image
But all seems okay when it is turned off.
image
Will also do the AYON test and update you later.

Copy link
Member

@moonyuet moonyuet left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also tested in AYON. I encountered same issue with redshift_rop but i dont think it's related to this PR.
image
Other than that, it looks good.

Copy link
Member

@moonyuet moonyuet left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I will approve this as it updates some latest tags in regard to the Houdini update.
The missing instance data of exportJob and iFdFiles need to be fixed for render_rops for sure but it is for different PRs.

@MustafaJafar
Copy link
Contributor Author

Also tested in AYON. I encountered same issue with redshift_rop but i dont think it's related to this PR. image Other than that, it looks good.

I think we should revise the logic implemented in this PR #5420.. I didn't expect that its logic will break other product types.

@moonyuet
Copy link
Member

Also tested in AYON. I encountered same issue with redshift_rop but i dont think it's related to this PR. image Other than that, it looks good.

I think we should revise the logic implemented in this PR #5420.. I didn't expect that its logic will break other product types.

It isn't. It's only breaking the render ROPs(farm cache looks okay) and we need to do an enhancement upon it for sure.

Co-authored-by: Roy Nieterau <roy_nieterau@hotmail.com>
@MustafaJafar
Copy link
Contributor Author

This shows that we should definitely label the job differently on whether it is:

  • Export Job
  • Render Job
  • Publish Job

In this case I can't really detect which of the mantra_ropSplit jobs is the export job and which is the render job.

yeah, I had to check plugin info to know which is which..

What about the render job's environment?

Also, can you also do a test run with a renderer that doesn't still render through Houdini?

E.g. Arnold, Redshift?

Since an Arnold render job wouldn't go through houdini logic it might not configure Arnold env correctly because it won't trigger houdini env modules, etc.

Maybe @moonyuet could help. could you give it a test in RS.

@moonyuet
Copy link
Member

E.g. Arnold, Redshift?

Since an Arnold render job wouldn't go through houdini logic it might not configure Arnold env correctly because it won't trigger houdini env modules, etc.

As I tested with Fabia's PR on export job with redshift for example, the rs files would be processed in the farm to render out the images. In other words they are using redshift standalone to render the job and once the renders finished, the rs files are gone and the rendered images would be located in publish folder

@MustafaJafar
Copy link
Contributor Author

This shows that we should definitely label the job differently on whether it is:

  • Export Job
  • Render Job
  • Publish Job

In this case I can't really detect which of the mantra_ropSplit jobs is the export job and which is the render job.

I think I've managed to fix it!

image

Also, I can find my published products as expected.
image

@BigRoy
Copy link
Collaborator

BigRoy commented Dec 14, 2023

This shows that we should definitely label the job differently on whether it is:

  • Export Job
  • Render Job
  • Publish Job

In this case I can't really detect which of the mantra_ropSplit jobs is the export job and which is the render job.

I think I've managed to fix it!

image

Also, I can find my published products as expected. image

The exported jobs - should not be published. So seeing you have "mantraifd" products that looks wrong. Those are intermediate rendering files, not publishes.

As I tested with Fabia's PR on export job with redshift for example, the rs files would be processed in the farm to render out the images. In other words they are using redshift standalone to render the job and once the renders finished, the rs files are gone and the rendered images would be located in publish folder

Correct, BUT environment variables required for plugins for redshift standalone and arnold standalone different from the ones for Maya.

So say you'd render with Arnold Standalone Render Plugin on Deadline we should now be setting the arnold standalone specific environment instead of the Maya Arnold Render Specific environment. Right?

As such, we might need a way to use a different environment for that particular render job than the environment from maya itself.

This might be less of a problem if when configuring the env for maya or houdini you're already also defining the environment for Arnold/Redshift standalone so that when during that render it uses Houdini's environment it also includes the vars required for Arnold standalone.


The next question of course is. What's currently implemented to ensure that the Arnold an Redshift render jobs render the correct version of Arnold and Redshift as defined in the project.

@MustafaJafar
Copy link
Contributor Author

The exported jobs - should not be published. So seeing you have "mantraifd" products that looks wrong. Those are intermediate rendering files, not publishes.

Sorry for inconvenience.. In my test, I published 4 things:

  1. Mantra with split enabled (intermediate rendering files are not published)
  2. Mantra with split disabled
  3. Karma
  4. mantra IFD (to check whether it is affected or not)

@antirotor
Copy link
Member

The next question of course is. What's currently implemented to ensure that the Arnold an Redshift render jobs render the correct version of Arnold and Redshift as defined in the project.

Correct me if I am wrong, but Arnold plugin on Deadline doesn't work with versions at all?

Anyway, what I am missing in case of Arnold is publish job.

image

@BigRoy
Copy link
Collaborator

BigRoy commented Dec 19, 2023

The next question of course is. What's currently implemented to ensure that the Arnold an Redshift render jobs render the correct version of Arnold and Redshift as defined in the project.

Correct me if I am wrong, but Arnold plugin on Deadline doesn't work with versions at all?

Anyway, what I am missing in case of Arnold is publish job.

image

That screenshot does also indeed seem to lack a publish job.

it should be:

  • Export job (export to a file format supported by a standalone renderer)
  • Render job (render the job with the renderer, e.g. arnold render, redshift render, husk)
  • Publish job (ingest the output frames from the render job)

The output files of the export job are considered intermediate files and are never published but only used by the render job.

Side note: Not sure how, aside of that, the publish logic behaves. But it's usually good practice to NOT delete the export job files and the render job files unless an explicit choice was made to do so. Because, when an issue occurs with e.g. a single frame or a few frames, then those will need to be re-rendered as opposed to re-rendering the full sequence. And then of course, once the frames are fixed, the publish job should be re-entrant to be able to publish again the fixed version (even if it had published before).

The above differentation about what each JOB type roughly is or does, and how it's intended to be used should be clearly worded in the documentation if it isn't yet.

Copy link
Member

@antirotor antirotor left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree with @BigRoy about the labeling of the jobs, also missing publish jobs are issue.

With Arnold ROP without job splitting, I have:

Traceback (most recent call last):
  File "C:\Users\annat\AppData\Local\Ynput\AYON\dependency_packages\ayon_2312141707_windows.zip\dependencies\pyblish\plugin.py", line 527, in __explicit_process
    runner(*args)
  File "C:\Users\annat\Documents\Projects\Ayon\OpenPype\openpype\modules\deadline\plugins\publish\submit_houdini_render_deadline.py", line 280, in process
    super(HoudiniSubmitDeadline, self).process(instance)
  File "C:\Users\annat\Documents\Projects\Ayon\OpenPype\openpype\modules\deadline\abstract_submit_deadline.py", line 471, in process
    render_plugin_info = self.get_plugin_info(job_type="render")
  File "C:\Users\annat\Documents\Projects\Ayon\OpenPype\openpype\modules\deadline\plugins\publish\submit_houdini_render_deadline.py", line 251, in get_plugin_info
    InputFile=instance.data["ifdFile"]
KeyError: 'ifdFile'

I thought that we've already fixed that...

@antirotor
Copy link
Member

I agree with @BigRoy about the labeling of the jobs, also missing publish jobs are issue.

With Arnold ROP without job splitting, I have:

Traceback (most recent call last):
  File "C:\Users\annat\AppData\Local\Ynput\AYON\dependency_packages\ayon_2312141707_windows.zip\dependencies\pyblish\plugin.py", line 527, in __explicit_process
    runner(*args)
  File "C:\Users\annat\Documents\Projects\Ayon\OpenPype\openpype\modules\deadline\plugins\publish\submit_houdini_render_deadline.py", line 280, in process
    super(HoudiniSubmitDeadline, self).process(instance)
  File "C:\Users\annat\Documents\Projects\Ayon\OpenPype\openpype\modules\deadline\abstract_submit_deadline.py", line 471, in process
    render_plugin_info = self.get_plugin_info(job_type="render")
  File "C:\Users\annat\Documents\Projects\Ayon\OpenPype\openpype\modules\deadline\plugins\publish\submit_houdini_render_deadline.py", line 251, in get_plugin_info
    InputFile=instance.data["ifdFile"]
KeyError: 'ifdFile'

I thought that we've already fixed that...

damn I am stupid.... wrong PR checked out, please disregard. Meanwhile, I'll kick myself.

@antirotor
Copy link
Member

antirotor commented Dec 19, 2023

anyway, publish job is missing:

image

top job is without job splitting, bottom one is with job splitting turned on. And the same applies to Arnold ROP

@MustafaJafar
Copy link
Contributor Author

MustafaJafar commented Dec 20, 2023

Member

I think this was solved already 😅
This is how it looks on my side.
image

I'm testing Arnold free version. I think it doesn't support farm render. but I can test render submission anyways.
image

@antirotor
Copy link
Member

Still no luck with the publish jobs,
image

@antirotor
Copy link
Member

I'm testing Arnold free version. I think it doesn't support farm render. but I can test render submission anyways.

btw you can use standalone arnold (or HtoA) on the farm if you set
image

@MustafaJafar
Copy link
Contributor Author

Still no luck with the publish jobs, image

on my side it works in production mode..
I've just tested in dev mode and I couldn't find publish jobs.
I'm trying to replicate it but I couldn't.

@iLLiCiTiT
Copy link
Member

NOTE: I think we should follow pyblish key naming camel case convention in instance.data and context.data. Thus "split_render" > "splitRender".

@antirotor
Copy link
Member

I've fixed my issue with bf0ad72 as it seems that submission_type was empty in my case - what I don't uderstand is how could this work before, because we are not setting it in houdini plugin anywhere. Nevertheless, we should remove Muster related thing asap.

@MustafaJafar
Copy link
Contributor Author

It works on my side.

Copy link
Member

@iLLiCiTiT iLLiCiTiT left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code LGTM

@antirotor antirotor merged commit 7128700 into develop Dec 21, 2023
5 checks passed
@antirotor antirotor deleted the bugfix/houdini_refactor_get_output_parameter branch December 21, 2023 13:38
@ynbot ynbot added this to the next-patch milestone Dec 21, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
host: Houdini size/S Denotes a PR changes 100-499 lines, ignoring general files target: AYON target: OpenPype type: bug Something isn't working
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

7 participants