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

Order of Param makes a difference. #377

Open
ggruszczynski opened this issue Nov 3, 2021 · 6 comments
Open

Order of Param makes a difference. #377

ggruszczynski opened this issue Nov 3, 2021 · 6 comments
Assignees
Labels
enhancement New features/updates

Comments

@ggruszczynski
Copy link
Member

I found that Order of Param in Model makes a difference.
I thought that only Order of <Boxes> in Geometry makes a difference.

My configuration:

./configure --enable-rinside --enable-double --enable-graphics --enable-cpp11 --with-cuda-arch=sm_75 

I was running with GPU SERIAL CUDA

[  ] -------------------------------------------------------------------------
[  ] -  CLB version:       v6.5.0-64-g018acb8b                               -
[  ] -        Model:       d2q9_pf_velocity_CM                               -
[  ] -------------------------------------------------------------------------

https://github.com/ggruszczynski/TCLB/tree/d2q9_pf_cm_p_and_v
(core as in develop)

I was running with:

CLB/d2q9_pf_velocity_CM/main example/multiphase/d2q9_pf_velocity/mrt_dam_break.xml 
CLB/d2q9_pf_velocity_CM/main example/multiphase/d2q9_pf_velocity/cm_dam_break.xml 

In
https://github.com/CFD-GO/TCLB/blob/develop/example/multiphase/d2q9_pf_velocity/mrt_dam_break.xml
https://github.com/CFD-GO/TCLB/blob/develop/example/multiphase/d2q9_pf_velocity/cm_dam_break.xml

Order of Param makes a difference. I get this error:

No water:

<Param name="PhaseField_init" value="1.0" zone="water"/>
<Param name="PhaseField_init" value="0.0"/>

Good:

<Param name="PhaseField_init" value="0.0"/>
<Param name="PhaseField_init" value="1.0" zone="water"/>
@mdzik
Copy link
Member

mdzik commented Nov 4, 2021

All TCLB XML is write-to-TLCB-as-you-read-XML, so yes - order matters. This is why warnings are displayed in your case (check output) We would need a DOM XML model, not a parser to handle it properly

@mdzik
Copy link
Member

mdzik commented Dec 7, 2021

To add bit more context:
#100

Without DOM model (build object tree then execute, implement LBM node's claims?) it's not easy to distinguish if a node was written too or not. There is warning in when you write to non-zero value for that case

IMHO One think that could be implemented is that Param without zone MUST always be triggered first, if not fail.
I could do that, but it might be breaking change for some examples. Votes?

BTW: XML standard assures that

<Param name="PhaseField_init" value="0.0"/>
<Param name="PhaseField_init" value="1.0" zone="water"/>

is different from

<Param name="PhaseField_init" value="1.0" zone="water"/>
<Param name="PhaseField_init" value="0.0"/>

Think of it as python's list of dicts. So it's predictable output.

@llaniewski
Copy link
Member

"Param without zone MUST always be triggered first" This is not such a bad idea. I vote yes

@TravisMitchell any comments?

@TravisMitchell
Copy link
Member

TravisMitchell commented Dec 8, 2021

I think a trigger for this is a good idea. If the user is overwriting their zonal value, I think we can assume it is not done on purpose and the code should flag this too them.

Vote: yes

@ggruszczynski
Copy link
Member Author

ggruszczynski commented Dec 8, 2021

What is/(shall be) the expected behaviour for the config below?

<Geometry nx="256" ny="256">
	<CM><Box/></CM>
	<None name="lava">Box ny="48" nx="32"/></None>
	<None name="water">Box ny="96" nx="32"/></None>
</Geometry>
<Model>
	<Param name="PhaseField_init" value="0.0"/>
	<Param name="PhaseField_init" value="1.0" zone="water"/>
	<Param name="PhaseField_init" value="-1.0" zone="lava"/>
</Model>

@llaniewski
Copy link
Member

@ggruszczynski <Param/> without zone= sets a setting in all the zones. So in your case: first all zones are set to 0, then water to 1 and lava to -1.

I think Michał's suggestion kind of makes sense, that you have to put all the <Param/> without zone= before the ones with zone=, and if you don't TCLB will give an error.

@llaniewski llaniewski self-assigned this May 3, 2022
@llaniewski llaniewski added the enhancement New features/updates label May 3, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New features/updates
Projects
None yet
Development

No branches or pull requests

4 participants