-
Notifications
You must be signed in to change notification settings - Fork 66
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
adding raja derived field #1161
Conversation
conduit::Node values; | ||
values = derived_field_add_reduction(dom[path1], dom[path2]); | ||
double *values = values["values"].value(); | ||
res[out_field].set(values); //need to preserve domain structure? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@cyrush blueprint question for getting my data back in my result node. What rules do I need to be following here? Do I need to maintain the domain structure for my output field? If not, am I overwriting my out field with each set? Should I instead loop over all domains append the new field to a vector and then set it at the end?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In this case, we can't pass const Node if we want to modify it (add a new field)
I think it would would be best is to return just the resulting Node field Node and insert it into the conduit tree at a higher level (in a custom filter)
We will also need to check association of the inputs (element or vertex) are the same and make sure they are both defined on the same topology. That info is in the field node along side the values.
f1:
values : [..]
topology : (string)
association: (string)
The resulting field needs to propagate that info.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah ok. This makes sense!
conduit::Node values; | ||
values = derived_field_add_reduction(dom[path1], dom[path2]); | ||
double *values = values["values"].value(); | ||
res[out_field].set(values); //need to preserve domain structure? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In this case, we can't pass const Node if we want to modify it (add a new field)
I think it would would be best is to return just the resulting Node field Node and insert it into the conduit tree at a higher level (in a custom filter)
We will also need to check association of the inputs (element or vertex) are the same and make sure they are both defined on the same topology. That info is in the field node along side the values.
f1:
values : [..]
topology : (string)
association: (string)
The resulting field needs to propagate that info.
const T val = l_accessor[i]; | ||
values[i] = val; | ||
}); | ||
ASCENT_DEVICE_ERROR_CHECK(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
to confirm my understanding:
If one field is larger than another, the output will be sized to the larger field and the remaining vals are simply copied.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Correct. That's what I was going for here, plus zero so simply copied what's extra. I figures it's ok to take them for different sizes? Or do I need to be concerned about topology further down the pipeline?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, yes this makes sense.
We aren't likely to hit these cases often b/c Blueprint fields on the same topology with the same assoc should share cardinality. But its much better to handle the logic vs having a real head scratcher crash down the line.
|
||
// resolve_symbol_result(graph(), output, this->name()); | ||
//set_output<conduit::Node>(output); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What should this output of the expressions filter be? I believe this is the output that gets written to the sessions file. I could output the new field? Or just metadata, like fields added and output field?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good question - the case were we paint the binning onto the mesh is the closest to this case, do we have a precedent there that makes sense?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good suggestion, I think that makes sense. I'll base it off that example.
} | ||
std::cerr << "dom before add_reduction" << std::endl; | ||
dom.print(); | ||
dom[output_path]["values"] = derived_field_add_reduction(dom[output_path], dom[path])["values"]; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Data is coming out here with device_values
|
||
// synch the values back to the host | ||
(void) field_sums.get_host_ptr(); | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@cyrush My domain is going into this functor with "values", and is coming out with a "device_values" added onto it.
ex:
rho_electrons21:
association: "element"
topology: "topo"
values: [0.0, 0.0, 0.0, ..., 0.0, 0.0]
device_values: [0.0, 0.0, 0.0, ..., 0.0, 0.0]
Have you seen this before? Am I messing something up here?
I was able to add a field to nothing and got correct results (as in the final image was correct, the output also had device_values). But then trying to add two fields together, it is as if it is only considering the second field. Not sure if it's pushing the input to "device_values" and then putting the fresh batch of values as "values"?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm, never mind. looks like device_values gets generated for the device calculations. So most likely unrelated as to why one field is overwriting the other.
my bad, need to guard my new derived add test on if rendering is available. |
@nicolemarsaglia FYI: I merged develop into this post my other exprs refactoring PR. |
Thanks! I appreciate it! |
b597893
to
1a48615
Compare
@cyrush ready for you to take a look at the raja/functor code. ascent/expressions/ascent_conduit_reductions definitely needs attention, it was renamed and reworked |
@nicolemarsaglia there were a few remaining puzzles, but I resolved them. Merging! |
@cyrush thanks for getting this wrapped up!! |
needed for warpx to add particle fields together before volume rendering