SCIInstitute / SCIRun

SCIRun is a Problem Solving Environment, for modeling, simulation and visualization of scientific problems. This is version 5, the upgraded version of SCIRun v4.
http://scirun.org
Other
125 stars 71 forks source link

Module porting: RefineMesh #681

Closed dcwhite closed 9 years ago

dcwhite commented 9 years ago
dcwhite commented 9 years ago

@benjaminlarson Notes added.

benjaminlarson commented 9 years ago

@dcwhite can you expand on the last item in the Module list? Little unsure what the interface algo layers are?

dcwhite commented 9 years ago

@benjaminlarson Referring to the mapping between GUI widgets (radio buttons/dropdowns) and the state variable replacement for guimethod. The association of names is unclear.

benjaminlarson commented 9 years ago

@dcwhite will these guards be the same with boost hashmaps?

https://github.com/SCIInstitute/SCIRun/blob/7a01bb33c8df040e4ed4423e742acbf527bab5c1/src/Core/Algorithms/Legacy/Fields/RefineMesh/RefineMesh.cc#L1357

benjaminlarson commented 9 years ago

If that is too time consuming to answer then I'll just leave them in for now.

a-y-khan commented 9 years ago

No, you'll need to get rid of all that old hash map code. The guards were needed to support the different hash map variants supported by different compilers on different platforms.

benjaminlarson commented 9 years ago

That makes it easier. Thanks.

dcwhite commented 9 years ago

Check out similar algorithm usage of hash map: https://github.com/SCIInstitute/SCIRun/blob/master/src/Core/Algorithms/Legacy/Fields/MeshDerivatives/GetFieldBoundaryAlgo.cc#L73

a-y-khan commented 9 years ago

It will be such a relief to get rid of that hash map mess :)

dcwhite commented 9 years ago

Good test network to replicate: SCIRun\src\nets\Modules\NewField\RefineMeshByIsovalue.srn Remove ConvertMeshToUnstructuredMesh, didn't seem necessary.

benjaminlarson commented 9 years ago

@dcwhite When I split up the RefineMeshAlgo I gave each algo a runImpl() method with it's class.... Do I declare this function const? If I do I get a bunch of type qualifying errors because a const function uses non const methods similar to : http://stackoverflow.com/questions/13103755/intellisense-the-object-has-type-qualifiers-that-are-not-compatible-with-the-me

Should I make all the functions in the runImp() const? I tried deleting const and eventually ran into a terminal "cannot instantiate abstract class" problem for each algorithm.

dcwhite commented 9 years ago

virtual run_generic() is const so everything else should be const too. Usually this is trivial since most Algorithm classes don't have any member data.

benjaminlarson commented 9 years ago

@dcwhite what is the equivalent for this line? https://github.com/SCIInstitute/SCIRun/blob/7a01bb33c8df040e4ed4423e742acbf527bab5c1/src/Core/Algorithms/Legacy/Fields/RefineMesh/RefineMesh.cc#L3548

I think it'll be like: bool convex = get_option(convex) == "hex_convex" but the convex option isn't from the GUI.

dcwhite commented 9 years ago

@benjaminlarson That's what I was referring to above in "You'll need to dig a bit to figure out the mapping between the "guimethod" variable and the interface/algorithm layers." You may have to debug the code to see how that boolean gets set--if it's always true or false and not settable from the GUI, then you can just always pass the constant value and remove the line, with a note that there's no way to set it for now.

dcwhite commented 9 years ago

Code merged, in testing stage now

benjaminlarson commented 9 years ago

@dcwhite I was following the other instances of the unordered_map: https://github.com/SCIInstitute/SCIRun/blob/master/src/Core/Algorithms/Legacy/Fields/MeshDerivatives/GetFieldBoundaryAlgo.cc#L73

When I was converting the hash map stuff I kinda knew it was off when I was looking at the iterators (locating and indexing). That module was probably a little too hard for me.

dcwhite commented 9 years ago

@benjaminlarson No problem--it was a good learning opportunity. It's good you recognized the off-ness--that's a skill too. Next time you can leave a note in a commit and I can look at it while you're working on it.