-
Notifications
You must be signed in to change notification settings - Fork 19
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
Implement Cartesian option as a config key (instead of the current compiler flag) #33
Comments
I think the transformation still makes sense in cases where the run is done in Milne but the vtk should be in cartesian |
Alright, I thought so too, then I'll fix the Cartesian VTK output first. Thanks! |
It is definitely a good idea! Are you thinking about it in the context of dynamical fluidisation?
If you have an idea how to do so, maybe we discuss it first. I certainly appreciate your readiness to do it (remember the manpower argument I mentioned in a different discussion)! With the existing and planned changes it might be actually worthwhile to make a publication about the 2025 code as compared to the 2014 reference I did back then. |
Okay, perfect. Thanks for your input! The dynamical fluidisation will benefit from this as well and I will discuss with Renan and Zuzana, although it's not my focus. I would only do the Cartesian config key implementation and leave further changes regarding the dynamical fluidisation to somebody else (if that is desired to have in I'll come back to this issue and start a discussion about how to tackle the testing and the implementation when I fixed the Cartesian VTK output. |
After handling the VTK source code fix, I started looking into the Cartesian option.
I went on to check a few things.
For my checks I used the following config and SMASH IC file. Maybe @yukarpenko and @zpauliny could look into this? Is it obvious to one of you why the value changes happen? And is this implementation (or more precisely the added lines) by Zuzana in commit 39cba1d something that should only be considered in the Cartesian runs or is this an improvement for Milne as well? |
I looked the commits up and yes, 39cba1d in Zuzana's |
@yukarpenko, thanks for the input! Then I will continue using Zuzana's branch for the implementation. |
I used the branch
cartesian_option
for some time now and it would be nice to refactor the Cartesian option from a compiler flag into a config key and eventually merge it back intostable_ebe
.VTK_Cartesian
) which is implemented right now in src/vtk.cpp in this case or should we get rid of it entirely?The text was updated successfully, but these errors were encountered: