AAVSO / VStar

VStar is a visualisation and analysis tool for variable star data brought to you by AAVSO
https://www.aavso.org/vstar
GNU Affero General Public License v3.0
9 stars 3 forks source link

Error while creating VeLa model #429

Closed mpyat2 closed 3 weeks ago

mpyat2 commented 3 weeks ago

1) Open a light curve, for example, V965 Cep, Unfiltered with V zeropoint, from 2459945.5 to 2460309.5, ObsCode PMAK. Convert to HJD (optional) 2) Create Fourier Model P=0.085067421, 6 harmonics (arbitrary) 3) Model -> Show Model -> Function. Copy function to clipboard. 4) Analysis -> VeLa model -> paste VelaCode. 4) Press OK. I see the following error: image

dbenn commented 3 weeks ago

I see where this is happening @mpyat2 and just working through the best/simplest fix.

I'm surprised this has not come up before now.

mpyat2 commented 3 weeks ago

Hi @dbenn Interestingly, this error does not come up in the "New star from VeLa model" observation source plug-in. Only in the VeLa Model tool plug-in.

dbenn commented 3 weeks ago

It relates to the observation environments being pushed in the model creator vs obs source.

dbenn commented 3 weeks ago

Note that running this (via Run button) in VeLa model dialog works fine:

zeroPoint is 48680

f(t:real) : real {
9.743964
+0.535862 * cos(2*PI*0.002295*(t-zeroPoint)) + 1.621261 * sin(2*PI*0.002295*(t-zeroPoint))
}

f(2447121.5)

Note that in this case there is the global VeLaScope and f's VeLaScope, but no VeLaValidObservationEnvironment (both subclasses of VeLaEnvironment<Operand>).

I'm about to commit:

They both work too, despite the presence of a VeLaValidObservationEnvironment.

dbenn commented 3 weeks ago

The reason neither the UT nor the plugin test fail is because when VeLaValidObservationEnvironment.lookup() is invoked, there is no newStarMsg defined because of the lifecycle of a test vs the whole system (observations loaded via Mediator):

https://github.com/AAVSO/VStar/blob/master/src/org/aavso/tools/vstar/vela/VeLaValidObservationEnvironment.java#L223

The block is skipped so columnInfoSource is not set by the calling test code. So, when this line is reached:

https://github.com/AAVSO/VStar/blob/master/src/org/aavso/tools/vstar/vela/VeLaValidObservationEnvironment.java#L91

the test code skips over it whereas this is not true when VeLaValidObservationEnvironment.lookup() is invoked normally from the VStar plugin.

Since what's being looked up at the point of failure is PI, an exception will be thrown when columnInfoSource.getColumnIndexByName(name) is called.

I can only assume that Fourier models were not used in the plugin before now, which I'm mildly surprised by. Either that or something else in the code of lookup() our downstream has changed but since any change to the class was 1 year ago vs the model creator code (7 months ago), that doesn't seem to be it.

Anyway...

The change is simple: catch the exception in lookup() returning an optional nullable, as we do in the UT and plug-in test cases.

dbenn commented 3 weeks ago

Commit message not specified properly on command-line, so repeated here:

now catching exception in VeLaValidObservationEnvironment.lookup()

See https://github.com/AAVSO/VStar/commit/205cc043359ef206a02683859353983c6e202d55 for change.

dbenn commented 3 weeks ago

@mpyat2 I see you approved the PR, thanks!