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

make unit specifier non optional #3

Open
dvc94ch opened this issue Jan 21, 2018 · 3 comments
Open

make unit specifier non optional #3

dvc94ch opened this issue Jan 21, 2018 · 3 comments
Labels
antlr-port Issues only applicable to our attempt to port to antlr

Comments

@dvc94ch
Copy link
Collaborator

dvc94ch commented Jan 21, 2018

What are your thoughts on units?

Is it ok to require the F in 10uF?

@kasbah
Copy link
Member

kasbah commented Jan 21, 2018

Like in #2 we have a bit of a conflict between search and HDL use cases. The existing implementation actually parses c 10u and capacitor 10u as 10uF but not 10u on it's own. For search cases we probably actually want to interpret 10u as a capacitor (what else could it be?). We also want to pick up on the capacitor word anywhere in the input (right now it needs to be at the start) a single c is a bit harder to decide on.

As an aside, when I have been playing around with HDL implementations I found the way it is currently kind of useful as you can have help function or sub-classes that do this for you: i.e. Capacitor('10u') == Component('capacitor 10u').

@dvc94ch
Copy link
Collaborator Author

dvc94ch commented Jan 21, 2018

how can you tell it's not an inductor from 10u? That could mean 10uH, or the recovery time of a diode 10us.

for passives we can match based on the unit. I think that it is natural to write the most important thing first, secondary information comes afterwards. So the first unit determines the component. With each component we can also have a secondary unit that comes after, and arbitrary units as key value pairs.

primary unit infered component component secondary unit other
resistance resistor power
capacitance capacitor voltage esr=resistance
inductance inductor current esr=resistance
frequency oscillator capacitance
voltage battery resistance

for active devices we need a different approach.

@kasbah
Copy link
Member

kasbah commented Jan 21, 2018

how can you tell it's not an inductor from 10u? That could mean 10uH, or the recovery time of a diode 10us.

Lol, yeah, knew I was forgetting something. 😆

I think that it is natural to write the most important thing first, secondary information comes afterwards.

These kinds of assumptions go out the window if we are thinking about search applications. People will write all sorts of things in any old order and we want to extract as much information as possible out of the input. There is a limit to what we can accept of course but not being too strict on the order of things seems like a really nice feature of the original implementation.

I think for the initial Antlr port, we should stick to the original and allow people to leave off the F if they add the word capacitor or cap or c at the start. We can then expand on this and allow pick up on the word anywhere (maybe even C if it's not part of degrees C).

That table is really useful though and I can see how allowing any order makes it harder to expand to different components. We might need to split off a "strict" grammar that allows many more components if we have issues expanding.

@kasbah kasbah added the antlr-port Issues only applicable to our attempt to port to antlr label Jul 31, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
antlr-port Issues only applicable to our attempt to port to antlr
Projects
None yet
Development

No branches or pull requests

2 participants