-
Notifications
You must be signed in to change notification settings - Fork 22
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
sigma of LSC2 class #137
Comments
Sounds good, let me take a look and I will get back to you soon. |
I am not sure what has changed but to use pyschism I had to:
|
I'm not sure either. I have pyschism with python 3.10 on SciClone, but I did the installation months ago. |
Hi Fei, I reviewed the code. Tell me if I understand what you mean. Now one can open an vgrid.in file with:
and return the sigma with: v_grid.sigma. One can also return nv with v_grid.nv, which is being defined by from_sigma. Are you saying you also need/want it to also return hsm, h_c, theta_b, theta_f? or we are good with returning the sigma values only? Please let me know if that is the case, by the way, I don't think |
Yes, I need LSC2.open to read an existing vgrid.in. I think this function has been there for a while. It didn't work yesterday because it tries to return cls(sigma). When reading an existing vgrid.in, we do not need to know or return hsm, h_c, theta_b, theta_f. Like you said, they cannot be derived if we only have vgrid.in. On the other hand, if we initialize LSC2 with def init(self, hsm, nv, h_c, theta_b, theta_f), it is perfectly fine to have these attributes exposed as v_grid.hsm, v_grid.h_c, etc. In addition, it is helpful to also have v_grid.nvrt and v_grid.sigma. Yes, I meant nvrt = sigma.shape[1], please help correct it. |
Yes, I committed a change yesterday to get LSC2.open working, which I referred to as "a temporary fix" in the original message. I added a factory method "from_sigma" and changed the return value from cls(sigma) to cls.from_sigma(sigma). Since you wrote most of the content for the LSC2 class, I thought you might prefer to structure it some other way as you see fit. If you don't see anything to change, the class works fine as is (beside the nv typo). Some additional thoughts: @josephzhang8 Please see if this makes sense. |
I didn't notice the flipping and I directly make a reference to sigma when initializing _snd. Okay, let me fix this as well as the nv typo on my side. |
Fei feel free to change the LSC2 class anyway you see fit! I really appreciate you letting me know but feel free to change it as much as need. Also feel free to ask me to change it if you are busy with other things - I will be glad to help you!!! |
Sounds good. Sorry if the original message was confusing, just want to be polite when I change others' code. |
@felicio93 @josephzhang8
Hey Felicio,
We noticed some issues with the new LSC2 class when running our existing scripts for generating nudging files.
The issue is that some scripts rely on the "sigma" attribute of an instance of the LSC2 class. I implemented a temporary fix. You can review it and make additional adjustment to the class as you see fit.
I see that you used a few parameters in "def init(self, hsm, nv, h_c, theta_b, theta_f)" and then calculate the sigma layers ("_snd"), which is a valid procedure. However, we also want to be able to read an existing vgrid.in without knowing how it is made (i.e., without knowing the parameter list of init ), and access something like "my_vgrid.sigma" later.
Thanks!
The text was updated successfully, but these errors were encountered: