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

Fixed the breaks by New version of ModelingToolkit #139

Merged
merged 10 commits into from
May 18, 2019
Merged

Fixed the breaks by New version of ModelingToolkit #139

merged 10 commits into from
May 18, 2019

Conversation

yashvardhan747
Copy link
Contributor

@yashvardhan747 yashvardhan747 commented May 4, 2019

Solves Issue #138

@yashvardhan747 yashvardhan747 changed the title fixed_the_breaks_for_new_version_of_ModelingToolkit Fixed the breaks by New version of ModelingToolkit May 4, 2019
@yashvardhan747
Copy link
Contributor Author

@JuliaRegistrator register()

@JuliaRegistrator
Copy link

Error while trying to register: Register Failed
@Yashcodes, it looks like you are not a publicly listed member/owner in the parent organization (JuliaIntervals).
If you are a member/owner, you will need to change your membership to public. See GitHub Help

@yashvardhan747
Copy link
Contributor Author

@JuliaRegistrator register()

@JuliaRegistrator
Copy link

Error while trying to register: Register Failed
@Yashcodes, it looks like you are not a publicly listed member/owner in the parent organization (JuliaIntervals).
If you are a member/owner, you will need to change your membership to public. See GitHub Help

- variables: Symbol[:x, :y, :z]
- expression: y() + z()
```
We can make object (of type `Separator` or `Contractor`)by just using function name (Note: you have to specify variables explicitly as discussed above when you make objects by using function name). We can also use polynomial function to make objects.
Copy link
Contributor

@mforets mforets May 11, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
We can make object (of type `Separator` or `Contractor`)by just using function name (Note: you have to specify variables explicitly as discussed above when you make objects by using function name). We can also use polynomial function to make objects.
We can make objects (of type `Separator` or `Contractor`) by just using the function name (Note: you have to specify variables explicitly as discussed above in this case). We can also use a polynomial function as shown below.

- expression: x() + y() == [-∞, 1]
```
#### BasicContractor
Object of type `Contractor` have four feilds (variables, forward, backward and expression), among them data of two feilds (forward, backward) are useful (i.e forward and backward functions) for further usage of that object, thats why it is preferred to use an object of type `BasicContractor` in place of `Contractor` which only contain these two feilds for less usage of memory by unloading all the extra stuff.(Note: Like object of `Contractor` type,`BasicContractor`'s object will also have all the properties which are discussed above).
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
Object of type `Contractor` have four feilds (variables, forward, backward and expression), among them data of two feilds (forward, backward) are useful (i.e forward and backward functions) for further usage of that object, thats why it is preferred to use an object of type `BasicContractor` in place of `Contractor` which only contain these two feilds for less usage of memory by unloading all the extra stuff.(Note: Like object of `Contractor` type,`BasicContractor`'s object will also have all the properties which are discussed above).
Objects of type `Contractor` have four fields (variables, forward, backward and expression), among them data of two fields (forward, backward) are useful (i.e. forward and backward functions) for further usage of that object, that's why it is preferred to use an object of type `BasicContractor` in place of `Contractor` which only contain these two fields for less usage of memory by unloading all the extra stuff. (Note: Like object of `Contractor` type,`BasicContractor`'s object will also have all the properties which are discussed above).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's a very silly mistake, thanks for pointing it out.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thats why it is preferred to use an object of type BasicContractor in place of Contractor

In which context is it preferred? Like, from the user perspective, for library code? I think that dwelling a bit into some of these details, or giving another example, would improve that paragraph.

Also i guess that the other two fields of Contractor do have a reason to exist and they are "useful" in some sense (at least internally). I think that this sentence could be improved a bit: "... only contain these two fields for less usage of memory by unloading all the extra stuff..".

Nice work!

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As I already mentioned in the paragraph, using BasicContractor is better because of less usage of memory and for less time complexity.
Other two fields are their just to give information about variables and expression used, which has no usage in contracting the interval w.r.t given constraint for which the object is used which is done by just using forward and backward function. Yes, Internally information about variables and expression is necessary after all only they are used to make forward and backward function but finally we just need these two functions. And these forward and backward functions are present in object .of type BasicContractor.

test/runtests.jl Outdated
@@ -59,10 +59,10 @@ end
C2 = Contractor(vars, y+z)
@test C2(A,X) == IntervalBox(0.5..1.5, 0.5..0.5, 0.5..0.5)

C1 = Contractor([:x,:y,:z], x+y)
C1 = Contractor([x,y,z], x+y)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add spaces: [x, y, z].

@test C1(A,X) == IntervalBox(0.5..0.5, 0.5..0.5, 0.5..1.5)

C2 = Contractor([:x,:y,:z], y+z)
C2 = Contractor([x,y,z], y+z)
@test C2(A,X) == IntervalBox(0.5..1.5, 0.5..0.5, 0.5..0.5)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You need to make this result be different from the previous one in order to check that x + y and y + z actually give different results.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is already shown that results are different for x+y and y+z.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

C1(A,X) and C2(A,X) are giving different results

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're right, sorry -- my mistake.

@dpsanders
Copy link
Member

You seem to have added extra docs files in the latest commit that I don't think should be there?

@yashvardhan747
Copy link
Contributor Author

@JuliaRegistrator register()

@JuliaRegistrator
Copy link

Error while trying to register: Register Failed
@Yashcodes, it looks like you are not a publicly listed member/owner in the parent organization (JuliaIntervals).
If you are a member/owner, you will need to change your membership to public. See GitHub Help

Project.toml Outdated
@@ -0,0 +1,25 @@
name = "IntervalConstraintProgramming"
uuid = "138f1668-1576-5ad7-91b9-7425abbf3153"
version = "0.1.1"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

0.11, not 0.1.1

@@ -0,0 +1,63 @@
<?xml version="1.0" encoding="UTF-8" standalone="no"?>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These are files that are generated in the build process and should not be included.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ohh. I'll delete them

@@ -59,6 +59,95 @@ julia> outer
([-100, 100],[-100, 100])
```

### Without using Macros
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

macros

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok.

Project.toml Outdated
IntervalContractors = "≥ 0.3.0"
IntervalRootFinding = "≥ 0.4.0"
MacroTools = "≥ 0.4.0"
julia = "≥ 1.0.0"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Aren't we requiring Julia 1.1? And ModelingToolkit 0.2

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh Right, Sorry its by mistake.
But Isn't for ModelingToolkit it should be autogenerated ?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess it's because you hadn't put a lower bound in to REQUIRE.
Just add it by hand.

@dpsanders
Copy link
Member

Did you see my comment about Project.toml and the versions we require?

@yashvardhan747
Copy link
Contributor Author

Did you see my comment about Project.toml and the versions we require?

Yes I did

@dpsanders dpsanders merged commit c60a5df into JuliaIntervals:master May 18, 2019
@dpsanders
Copy link
Member

Thanks!

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

Successfully merging this pull request may close these issues.

None yet

4 participants