jktebop wrapper support for exposure times

[kecnry]

No description provided.

[jsinkbaek]

Repeat of question from #699

I don't see any reference to dataset in the JKTEBOP backend code. Is that handled neatly such that the bundle there only sees one dataset (the one it needs)? Or will I run into trouble there if it is defined like a regular parameter, in case there is two light curves defined?

[kecnry]

Each dataset has an exptime parameter:

phoebe2/phoebe/parameters/dataset.py

Line 241 in 05e263b

params += [FloatParameter(qualifier='exptime', value=kwargs.get('exptime', 0.0), default_unit=u.s, description='Exposure time (time is defined as mid-exposure)')]

and then the compute options for each backend that supports exposure times has an fti_method and fti_oversample parameter defined which will each automatically duplicate for each dataset:

phoebe2/phoebe/parameters/compute.py

Lines 182 to 183 in 05e263b

	params += [ChoiceParameter(qualifier='fti_method', copy_for = {'kind': ['lc'], 'dataset': '*'}, dataset='_default', value=kwargs.get('fti_method', 'none'), choices=['none', 'oversample'], description='How to handle finite-time integration (when non-zero exptime)')]
	params += [IntParameter(visible_if='fti_method:oversample', qualifier='fti_oversample', copy_for={'kind': ['lc'], 'dataset': '*'}, dataset='_default', value=kwargs.get('fti_oversample', 5), limits=(1,None), default_unit=u.dimensionless_unscaled, description='Number of times to sample per-datapoint for finite-time integration')]

Since the jktebop backend currently creates a separate call to jktebop for each defined dataset (see info['dataset'] in the _run_single_dataset method), you should be able to just pull those three parameters for the current dataset and pass them on as necessary (or oversample in jktebop and then recombine within the wrapper).

[jsinkbaek]

Ah so I don't even need to touch _worker_setup() like I did before?

[kecnry]

I don't think so, for the jktebop wrapper (it varies a bit between the different wrappers since some can run multiple datasets in a single call, etc) I would think of _worker_setup as things that apply to all datasets (system parameters, general compute options, etc), and _run_single_dataset as anything dataset-specific.

[jsinkbaek]

Makes sense, it looks like I can get it to run by getting values from the compute_params in _worker_setup(), but that does not seem very intuitive (or correct). So since it looks like parameters like fti_oversample are only exposed if fti_method == 'oversample', I don't need the third parameter and can just try directly in _run_single_dataset() instead

[jsinkbaek]

At the moment, it looks like there is some issue with "2 results found" when I try to implement this the simple way. I will get an error like:

ValueError: 2 results found: ['fti_oversample@lc01@phoebe01@phoebe@compute', 'fti_oversample@lc01@compute1@jktebop@compute']

Then, if I try to require in the code that it uses either compute=compute (where compute would be compute1), context='jktebop', or anything else defining, it will throw an error stating that no parameter was found with that twig.

[kecnry]

Is fti_method set to 'oversample'? If you want to pull the value even if it is not, you can pass check_visible to your filter or get_value calls. If that still doesn't work, can you push what you have to a branch on your fork and I might be able to see what is causing the problem.

[jsinkbaek]

It should be, but I will check again tomorrow.

…

On Tue, 14 Feb 2023, 17.27 Kyle Conroy, ***@***.***> wrote: Is fti_method set to 'oversample'? If you want to pull the value even if it is not, you can pass check_visible to your filter or get_value calls. If that still doesn't work, can you push what you have to a branch on your fork and I might be able to see what is causing the problem. — Reply to this email directly, view it on GitHub <#700 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ANR5GXBO6SQ7DLEMIVC6P3LWXO56PANCNFSM6AAAAAAU3ZAEQU> . You are receiving this because you commented.Message ID: ***@***.***>

[jsinkbaek]

Hi @kecnry, I messed up my fork by pushing the rv bugfix to master. So is it ok if I just make a branch with both the rv bugfix and the fti thing I have attempted? And then you can just look at what I have done and implement something in a different branch?

[jsinkbaek]

Hi again, I reverted the master branch and made a new branch "jktebop_fti_oversample" after. Check it out if you want to see the implementation. It seems to work as intended now, but I was forced to use twigs to get it to work.

It also uses a try except statement, since I could not figure out an intuitive way to check if the fti_oversample parameter exists.

[kecnry]

Thanks! I opened that branch as a draft PR (#702) so that it's easier for anyone to update, and also add a list of things that I think we'll need to address (feel free to do any you would like and just push to that same branch, or we can take over at any point and try to wrap it up - just let me know what you'd like to do).

[jsinkbaek]

I think I would prefer if you all took over. I wrote it with twigs and try/except because I could not get the filtering to work and saw no other way intuitively, despite knowing that it was considered non-ideal. So those hacks show the limits of my comprehension of the internals of PHOEBE.

[kecnry]

Ok, will do. It's probably not terribly high on our priority list if the version you have locally works for what you need, but we'll try to wrap it up as soon as we can. Feel free to subscribe to the PR if you want to be notified of future work/conversations there. Thanks for getting it this far!

[jsinkbaek]

It should be completely functional, and it looks like it is working on my local install.