Skip to content
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

Incorrect peak selection for 'max' ignition type #22

Open
vayner-s opened this issue Jun 23, 2021 · 0 comments
Open

Incorrect peak selection for 'max' ignition type #22

vayner-s opened this issue Jun 23, 2021 · 0 comments

Comments

@vayner-s
Copy link

vayner-s commented Jun 23, 2021

It seems like the 'max' ignition-type case is not working as intended.

def get_ignition_delay(time, target, target_name, ignition_type):

    if ignition_type == 'max':
        # Get indices of peaks
        peak_inds = detect_peaks(target, edge=None, mph=1.e-9*np.max(target))

        # Get index of largest peak (overall ignition delay)
        max_ind = peak_inds[np.argmax(target[peak_inds])]

        #ign_delays = time[peak_inds[np.where((time[peak_inds[peak_inds <= max_ind]]) > 0.0)]]

        ign_delays = time[peak_inds[peak_inds <= max_ind]]

I'm not sure if ign_delays is meant to include all of the peaks up to and including the max peak?
Later on in:

def process_results(self):

        if max_temperature >= init_temperature + 50.:
            ignition_delays = get_ignition_delay(time.magnitude, target,
                                                 self.properties.ignition_target,
                                                 self.properties.ignition_type
                                                 )
            self.meta['simulated-ignition-delay'] = (ignition_delays[0] - time_comp) * units.second

The first value in ignition_delays is always used, so the delay for the max peak doesn't actually end up being selected unless it happens to also be the first peak. From looking at the code, it seems like the same issue may be occurring for the 'd/dt max' ignition type as well, but this hasn't yet come up in testing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant