Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Core][VTK] make vtk default settings static #3871

Closed
wants to merge 1 commit into from

Conversation

armingeiser
Copy link
Member

@armingeiser armingeiser commented Jan 17, 2019

As it is done in the GiDOutputProcess.

@adityaghantasala
Copy link
Member

curious ! Why is this required ?

@armingeiser
Copy link
Member Author

armingeiser commented Jan 17, 2019

It is helpful if you wan't to look at the defaults without/before creating an instance.
E.g. in the ShapeOptimizationApplication we wan't to change some of the default settings. So what we can do like this, is to get a copy of the static variable, modify the settings and call ValidateAndAssignDefaults with those modified settings.
Then we create the actual instance of the vtkio.

tmp_default_parameters = VtkOutputProcess.default_parameters.Clone() #important to Clone so the originals do not get overwritten
tmp_default_parameters["folder_name"].SetString("Optimization_Results")

current_parameters.ValidateAndAssignDefaults(tmp_default_parameters)

vtk_io = VtkOutputProcess(model, current_parameters)

Like this we do not duplicate default settings, but just reuse them from the place where they are originally defined.

@adityaghantasala
Copy link
Member

I see ... I am not against but concerned that this is not consistent with the way processes are doing right now !

@philbucher
Copy link
Member

Hm I think this might be a blocker for #3766
We are trying to find a common format for the output processes

@armingeiser why don't you do it manually?

if not user_defined_settings.Has("folder_name"):
    user_defined_settings.AddEmptyValue("folder_name").SetString("Optimization_Results")

This is also what I do in similar situations

@philbucher
Copy link
Member

maybe also a function AssignDefaults() (without Validation) could do the job, I would have like this also in other cases already

now that #58 is merged we can again work on the Parameters

@armingeiser
Copy link
Member Author

armingeiser commented Jan 18, 2019

@philbucher of course i could do it manually, but doing this for more than one setting or even nested settings (as for GiD) is very tedious.

AssignDefaults() could probably also do the job.

@armingeiser
Copy link
Member Author

I created #3889 as suggested by @philbucher.

@philbucher
Copy link
Member

do you still need the branch?

@armingeiser armingeiser deleted the core/make-vtk-defaults-static branch February 14, 2019 13:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants