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

[V6.1 RC] Fixes #101 and new NVCC required option #123

Merged
merged 3 commits into from
Dec 19, 2016

Conversation

mdzik
Copy link
Member

@mdzik mdzik commented Dec 16, 2016

xmlint not is ok with config, and:

 +NV_OPT = -D_FORCE_INLINES 

fixes compilation issues on newer GCC/NVCC
after
BVLC/caffe#4046

@llaniewski llaniewski added this to the V6.1 Release Candidate milestone Dec 16, 2016
@llaniewski llaniewski self-requested a review December 16, 2016 14:12
@@ -19,7 +22,8 @@ int MainContainer::Init () {
c.append_attribute("cross").set_value("GPU");
#endif
solver->configfile.save_file(filename);
Copy link
Member

Choose a reason for hiding this comment

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

You cannot write the xml file after the execution, because the execution can fail, or can be stopped - and you would still want the xml file to be created.

Copy link
Member Author

Choose a reason for hiding this comment

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

BUT we don't want to process altered XML.
I might add save of deep copy instead of solver->configfile directly

Copy link
Member

Choose a reason for hiding this comment

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

I see no problem in processing the altered XML. Maybe even in future we will want to add things to the XML on-the-go (by python for instance)

@@ -204,16 +204,17 @@ vHandler * getHandler(pugi::xml_node node) {
ret = new acSyntheticTurbulence;
} else if (name=="Run") {
output("Skipping 'Run' element");
}else {
ret = new Callback();
Copy link
Member

Choose a reason for hiding this comment

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

Why generic Callback? I think it should be NULL. If there is segfault because of it - this should be solved, where the segfault occurs.

Copy link
Member Author

Choose a reason for hiding this comment

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

because: https://github.com/CFD-GO/TCLB/blob/v6.1_candidate/src/Handlers.h#L17

It does fault with NULL pointer. I think that returning NULL where pointer-to-object is expected is design error.

I could change it to

		if (hand != NULL) {
     		        hand->solver = solver_;
			int ret = hand->Init();
			if (ret) {
				delete hand;
				hand = NULL;
			}
		}
		ref = new int;
		*ref=1;
debug0("Handler shared pointer: create\n");

but other methods of Handler might expect it to be valid object somewhere

Copy link
Member Author

Choose a reason for hiding this comment

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

Maybe NullCallback or something? To have consistent return data type

Copy link
Member

Choose a reason for hiding this comment

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

Ok, please add a NullHandler then.

@llaniewski llaniewski changed the title fixes #101 and new NVCC required option [V6.1 RC] Fixes #101 and new NVCC required option Dec 19, 2016
@llaniewski llaniewski merged commit 91fbc55 into CFD-GO:v6.1_candidate Dec 19, 2016
@llaniewski
Copy link
Member

Closes #101

@mdzik mdzik deleted the fix_101 branch December 19, 2016 13:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants