Mandatory REL file to draw SRF planes, check if fully inside VM domain

[sungeunbae]

Previously, when plot_vm.py is executed as a separate script, it would only plot VM domain without considering SRF.
This behaviour is not consistent with a case where plot_vm() is called during rel2vm_params stage. In this case, it passes srf_corners and it will also find srf.path file in the temp directory, so SRF planes are plotted alongside the VM domain.

I fixed this with additional rel_path argument for plot_vm.py. This is now a mandatory, and with the CSV file supplied, it can work out SRF corners and plot the SRF planes.

It will also call validate_vm_bounds() to see if the VM domain fully encloses the SRF corners, which was not done previously.

[lispandfound]

Instead of manual parsing yourself, an easier way would be StringIO (from the io module) plus np.loadtxt

Suggested change

	[
	(float(lat_str), float(lon_str))
	for lon_str, lat_str in [
	point_str.split("\t")
	for point_str in vm_corners.split("\n")
	if len(point_str) > 0
	]
	]
	np.loadtxt(StringIO(vm_corners))

[sungeunbae]

Nice suggestion. Need to swap the order of lon and lat, so

    vm_polygon = Polygon(
        np.loadtxt(StringIO(vm_corners))[:,[1,0]] # [lon, lat] to [lat, lon]
    )

[AndrewRidden-Harper]

I don't have any specific comments on the code. However, regarding your question about whether it should give a warning of fail hard, my preference would be for it to fail with an error code (after it has made the plot so the user can see what's wrong)

[joelridden]

Does this need to be in the function?

[sungeunbae]

I admit this is a hack. But to avoid a cyclic import, I initially attempted some refactoring of the repo, but it was becoming too large.

[sungeunbae]

I noticed validate_vm.py already has validate_vm_bounds() that was supposed to do the check. During the vm_params step, this check is done and returns "failed" status for the vm_params step. However, if we run plot_vm.py as a separate script, this check was never done.

Additionally, I was not very happy with validate_vm_bounds(), so made a separate change: https://github.com/ucgmsim/qcore/pull/333/files

[sungeunbae]

I don't have any specific comments on the code. However, regarding your question about whether it should give a warning of fail hard, my preference would be for it to fail with an error code (after it has made the plot so the user can see what's wrong)

I don't have any specific comments on the code. However, regarding your question about whether it should give a warning of fail hard, my preference would be for it to fail with an error code (after it has made the plot so the user can see what's wrong)

I had a look at the bigger picture of the workflow and realized that "fail with an error code" is already implemented as a part of VM params step of the workflow. https://github.com/ucgmsim/slurm_gm_workflow/blob/757e701a05b8160d3fd95fdb4aedcfc049e0d50f/workflow/automation/org/nesi/vm_params_gen.sl#L60

There is validate_vm_bounds() function that checks the domain, which returns FAILED to the workflow manager. (but that function needed some work. See https://github.com/ucgmsim/qcore/pull/333/files)
Such a check was never made when plot_vm.py was used as a stand-alone script, and it is the gap I'll trying to fill in with this PR.

If we limit the scope of this PR to "make plot_vm.py more useful as a stand-alone script", I think a warning message is good enough.

[sungeunbae]

@claudio525 @lispandfound Any comment, please?