search

SENSEI-insitu / SENSEI

SENSEI ∙ Scalable in situ analysis and visualization
https://sensei-insitu.org
Other
22 stars 21 forks source link

Added scriptversion parameter to Catalyst analysis #100

Closed jwindgassen closed 1 year ago

jwindgassen commented 1 year ago

When using Catalyst Analysis, ParaView will try to determine the Version of the passed Catalyst script by checking the header of the file for a # script-version: 2 line (see here). If it does not exist, it will default to version 1. With the scriptversion="2" attribute, the user can now overwrite that default version. While ParaView will always generate that line when exporting a Catalyst script, if you modify of create your own script and don't know about this mechanic you will run into errors.

Note that the # script-version: 2 line in a Catalyst script will take precendence over this attribute and it merely presents a fallback version for Catalyst.

burlen commented 1 year ago

@patrickoleary @c-wetterer-nelson would you be willing to give this improvement to our Catalyst support a quick review? @ghweber fyi

burlen commented 1 year ago

@jwindgassen thank you

c-wetterer-nelson commented 1 year ago

Hi @jwindgassen thanks for the PR! I like this in general, helps the power users who aren't generating their scripts directly in paraview. We obviously urge users to start with a ParaView generated script, to avoid problems like this, but we understand that users have their specific use cases!

I'll note that the version checker also checks the paraview major and minor version listed in the preamble of catalyst scripts, and does a couple other checks to figure out what version it is dealing with. All of those checks can be evaded by a hand written script of course.

c-wetterer-nelson commented 1 year ago

This is definitely a "use at your own risk" feature. Relying on this could lead to challenges debugging non-compliant scripts.

jwindgassen commented 1 year ago

I agree, that it might be a confusing option if you rely on it being the actual version that is chosen. But since the comment in the script (or any of the other checks ParaView does) always takes precedence, I think the actual chance of any conflicts are pretty small. Maybe calling the option version_hint instead of script_version might be more purposeful here.

c-wetterer-nelson commented 1 year ago

I like version_hint! that sounds good to me.