search

pshriwise / openmc

OpenMC Monte Carlo Code
http://openmc.readthedocs.io/
Other
5 stars 2 forks source link

updated defaults #15

Closed eepeterson closed 2 years ago

eepeterson commented 2 years ago

Just taking a look at a few things and suggesting some tweaks. I'm hoping to also add the ability to have the 3 allowed filter types in any order.

eepeterson commented 2 years ago

also does it make sense to open a tiny PR to develop with the change to src/weight_windows.cpp so this branch only has the 1 file diff?

pshriwise commented 2 years ago

Just taking a look at a few things and suggesting some tweaks. I'm hoping to also add the ability to have the 3 allowed filter types in any order.

I thought about doing that here, but decided I would just do that as part of the C++ API call. I can look into it though.

One thing I considered was just reordering the filters before starting the run, but then the tally passed in wouldn't match the tally in the statepoint, which might be confusing and cause problems. Maybe with a warning that would be okay though? 🤔

eepeterson commented 2 years ago

I'd prefer if the tally and filters are the same in the set up and statepoint. I think we could do any necessary reshaping or processing to get the weight windows in the correct format internally. For example have a WeightWindow.from_flux_tally method or something equivalent on the C++ side. All the logic for dealing with filter ordering could be in there so regardless of what order the user specified, their weight windows will come out correct.

pshriwise commented 2 years ago

also does it make sense to open a tiny PR to develop with the change to src/weight_windows.cpp so this branch only has the 1 file diff?

Yep, can do.

eepeterson commented 2 years ago

If you're ok with the above strategy we could prototype it on the python side in this script and then implement it in C++. This functionality would also make it very easy to update ww on the fly under the hood by batches or something.

pshriwise commented 2 years ago

I'd prefer if the tally and filters are the same in the set up and statepoint. I think we could do any necessary reshaping or processing to get the weight windows in the correct format internally. For example have a WeightWindow.from_flux_tally method or something equivalent on the C++ side. All the logic for dealing with filter ordering could be in there so regardless of what order the user specified, their weight windows will come out correct.

That's pretty close to what I'm headed toward on the C++ side as we speak. Coding it up right now.

pshriwise commented 2 years ago

If you're ok with the above strategy we could prototype it on the python side in this script and then implement it in C++. This functionality would also make it very easy to update ww on the fly under the hood by batches or something.

Totally down with a prototype on the Python side. I have a good idea of what needs to happen. It may be a little tricky to implement with xtensor since not every NumPy method/index is directly translatable, but I'm confident it's possible.

pshriwise commented 2 years ago

I'm going to go ahead and merge this. If you'd like to submit a follow-on to this to allow arbitrary filter ordering, go for it!

I was fairly close to this here https://github.com/openmc-dev/openmc/commit/30fa4f20c6c526e94173cfb2d1925be70be08cc9, but again decided not to fuss with it further in favor of focusing on the longer-term implementation on the C++ side.